AaronBallman wrote: > > > I don't think the discussion here has run its course > > > #115416 > > > > > > Yeah, I had explicitly asked for a review from @Bigcheese before that > > landed, so I was surprised to see that get merged. > > I misread it. I thought it wasn't message to me. It would be clear if you ask > me for waiting more time for @Bigcheese
I'll keep that in mind for next time! > > > I don't think we should force users for our too conservative decisions. > > > As I said there are many false positive error messages which doesn't > > > affect the process actually. > > > > > > False positive _error_ diagnostics? Warnings can have false positives; > > errors are not allowed to and that suggests we have some serious issues > > elsewhere to address first. > > Yes. This was my point. Maybe I didn't make it clear. All the `LANGOPT` in > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LangOptions.def > now are not allowed to be inconsistent in different TUs when importing. But > actually some of these flags won't affect things. We changed some of them, > e.g., VisibilityMode ( > > https://github.com/llvm/llvm-project/blob/700d9ac9ef82fa5aa6b2972e8656ab5055a90d15/clang/include/clang/Basic/LangOptions.def#L380-L381 > ). But giving the amount of options, there must be a lot of too-conservative > `LANGOPT`. I see, so because we no longer allow inconsistent options when importing, we now issue a lot of error diagnostics. But for many of the options, differences between TUs should have no impact on modules anyway, and so those errors are false positives. Is that about correct? If I'm right, then what about a different way forward: update the individual options to specify whether it is an error, a warning, or silently fine for the option to be inconsistent across module boundaries, the emit diagnostics (or not) as appropriate for each option. It's more up-front work, but it means we stop issue false positive *errors* without losing diagnostic fidelity. (Note, we may find we want to reorganize Options.td so it's easier for us to specify "this entire block of options are ones for which module inconsistency is an error".) https://github.com/llvm/llvm-project/pull/117840 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
