[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
Tue Oct 15 03:02:01 PDT 2024


ilya-biryukov wrote:

> 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.)

We would need a way to enable or disable this diagnostic internally, ideally via a runtime switch, so that we don't need to ship a new compiler every time a user hits this warning. I think the same also holds for users that use RC: we want them to report the bug, but they also need to make progress on their testing of RC/whatever else they're doing with it.
The only downside of having a diagnostic group that I see is that it's public and shows up in documentation everywhere. However, given the amount of warnings we have, maybe that's not even a big deal.

I think the way out of it is to enable this diagnostic only after we make sure it is not chatty. So we do a gradual rollout:
- add a warning, disabled by default,
- collect cases where we cannot demangle in important projects by enabling the diagnostic for a single run,
- fix those problems,
- enable the warning by default up to RC builds as proposed,
- collect and fix any problems that happen from RC,
- enable the warning by default in release because it's not chatty anymore.
- (if the demangler is really stable) remove the group so that the diagnostic cannot be disabled.

Hopefully by the last step, we will be discovering almost all names that we cannot demangle during Clang development and users will almost never see it.
It it's rare, I hope this would motivate the users to disable it less (although some still will). If we really want to fight that, we can actually remove the group and force it to be enabled by default.

+1 to having this discussion in the RFC

https://github.com/llvm/llvm-project/pull/111391


More information about the cfe-commits mailing list