Thoughts on having a header to contain default values for different options?

I was looking at this PR and one thing that came to mind is that default values for various options are kinda' spread across the code. It seems a bit dissatisfying. So I was thinking that maybe we should lift the constants out into a header. I'll just copy-paste one of PR comments:

[what about] having something like say a header named swift/Basic/DefaultOptions.h which has stuff like

namespace swift {
namespace default {
static constexpr bool EnableFineGrainedDependencies = false;
// ... other default options.
}
}

And all the places which use it refer to it as default::EnableFineGrainedDependencies. I think we should consult other people on what they think, since if we start adopting such a style it would affect other parts of the code too.

The solution you've taken here also works, in that it achieves the same goal of avoiding duplicate magic constants. However, I feel that it ends up creating slightly awkward looking statements like

  Opts.EnableFineGrainedDependencies =
      Args.hasFlag(options::OPT_enable_fine_grained_dependencies,
                   options::OPT_disable_fine_grained_dependencies,
                   Opts.EnableFineGrainedDependencies);

where one needs to have the surrounding context that "Opts was default initialized before, and we are setting this parameter for the first time, not resetting it based on an 'old' value". Instead, if it were written like

 Opts.EnableFineGrainedDependencies =
      Args.hasFlag(options::OPT_enable_fine_grained_dependencies,
                   options::OPT_disable_fine_grained_dependencies,
                   default::EnableFineGrainedDependencies);

it doesn't require any surrounding context since convention dictates that all default::Foo values are const/constexpr.

What do people think of this idea? Yay/eh/nay?

One downside is that it is not clear what the default value is at the point where the EnableFineGrainedDependencies field is specified, since it would just read like bool EnableFineGrainedDependencies = default::EnableFineGrainedDependencies;.

Terms of Service

Privacy Policy

Cookie Policy