[Incremental] Use the initial value of EnableFineGrainedDependencies as the default.

[davidungar]

Following up on a thought by @varungandhi-apple , centralize the default for enabling fine-grained-dependencies.

[davidungar]

@swift-ci please smoke test

[davidungar]

@swift-ci please smoke test

[varungandhi-apple]

Ah I wasn't clear in my comment last time. What I meant in that PR was 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.

[davidungar]

@swift-ci please smoke test

[davidungar]

Ah I wasn't clear in my comment last time. What I meant in that PR was 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.

Thanks again! I agree about that there's some awkwardness here, but the ability to disable a somewhat risky change in a single place outweighs the awkward factor for me.

The "default" proposal interesting. It smacks of the challenge we face when we try to project a multidimensional problem down into a one-dimensional containment hierarchy. The projection forces us to privilege one dimension above all others.

[davidungar]

@swift-ci please smoke test os x platform