tarunprabhu wrote: > Thanks for the review. > > I wonder if my mistake here could be a reason to change the policy. It is not > obvious that after changing a default value in a configuration structure, one > then has to change code handling flags in an entirely different subproject. > Then the failure mode when such a change is forgotten is particularly bad - > in this case the negative flag was silently ignored. I think in future new > options should follow the structure I use here: if any flag is given: forward > the last one unconditionally, without encoding the default in two places. > That will be easier to maintain.
Presumably the default in the configuration structure is also the default that a user would expect. They may not be changing their use of the compiler, but the documentation would (or should) be clear about what the default is, so the change in configuration does filter all the way to the top, in a different project. For enum-valued configuration options, one would have to change the driver (or the frontend) in such cases so the correct default gets passed along. It's a bit odd for booleans because the default is not explicit, but implicit in the presence or absence of a command-line flag. We should avoid replicating options in the driver and the frontend if possible. In flang, we have made more options visible in both than we probably should have, but, IMO, we should follow clang's approach and limit what gets passed to the frontend. That would allow us to share more option validation code with clang - as we have been doing more frequently of late. I think clang also mitigates some of these issues by automatically marshaling some command-line options, especially booleans directly into the underying configuration structures, whereas we don't. It may be worth seeing if we could do that. This way, we may be able to avoid having to manually change things in multiple places - simply changing the default in command-line option specification would be sufficient. https://github.com/llvm/llvm-project/pull/217266 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
