ilya-biryukov 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, - collect issues notable 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, - enabling 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). 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
