AaronBallman wrote: > > I agree completely. I have a preference for the second thing as well. We > > can have this be a warning-as-default-error type thing, which allows us to > > disable this with a flag, but is still an error. I MIGHT suggest using the > > functionality to see if that diagnostic is on before doing the test (due to > > how much additional work there is here), but IMO, that is a much better way > > forward. > > 👍 I guess that's a good signal that this is the way to go. AFAIK, @VitaNuo > has already started exploring this direction. Wrt to enabling this warning, I > think we should probably start by having this warning disabled because there > seems to be too many failures, but we could gradually roll it out: > > * start with a warning that's disabled by default, > > * collect issues that we hit in LLVM tests (there's plenty AFAIU) and > also from real projects (we can run with the warning internally over Google's > codebase to get a representative sample), > > * fix all demangling issues present in the tests and the most common ones > from other projects, > > * enable the flag by default. > > > @erichkeane how do you feel about having it enabled given that this does not > really block users, but merely points at demangling issues in LLVM's > demangler (that they don't necessarily even use). It feels like this should > be internal, but then we won't get any bug reports. I feel like we should at > least fix the majority of the issues before enabling it (hence the rough > roadmap above).
I am not super comfortable with enabling the diagnostic by default because it's not something the user can do anything about aside from disable the diagnostic (which means they'll report one issue and probably never report another again because they disabled the warning due to chattiness) and I don't think we want to train users to ignore diagnostics or force them to turn off low-value ones. However, what about this as a compromise: we enable the diagnostic without a diagnostic group (so it cannot be disabled) in all builds up to the release candidate, and then we fully disable it for the actual releases? This way, early adopters can help us find issues, but we're not inflicting any pain on the general public once we release. (FWIW, I think this is something that should have an RFC for wider community buy-in given the effects.) https://github.com/llvm/llvm-project/pull/111391 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
