erichkeane 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 definitely have some heartburn about the enabling-of-flag by default, I would expect these diagnostics to be something actionable by our users, but I also definitely see the wish to get as many people testing this to improve the llvm demangler as possible. I pinged a few others to comment here, but I'd have to be reasonably sure this is a 'no one will ever see this' kind of thing, and if they DO, have a good idea of what to do. I'm on the fence, but if we were to, I have some thoughts: 1- Can we test this on the top X github C++ projects as well? Perhaps at LEAST Boost. 2- Can we ensure that anyone who searches for the diagnostic has a way of VERY QUICKLY seeing how to report it? So it has to be VERY well documented. 3- Have the diagnostic make it clear that this isn't a "something wrong with your code, but with our demangler"? 4- Instead of being a warning, how about making it a 'remark'? 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
