[clang] [llvm] [clang][LLVM Demangler] Add an assertion that validates that all mang… (PR #111391)

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 10 06:51:03 PDT 2024


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


More information about the cfe-commits mailing list