[clang] [clang] [diagnostics] Stable IDs for Clang diagnostics (PR #168153)

[Dave Bartolomeo via cfe-commits]

dbartol wrote:

@AaronBallman @erichkeane

Both of you make good arguments for keeping error IDs stable, so I'm on board with that. I don't think that decision changes this PR or the tblgen PR I linked to, since the new properties are supported on any `Diagnostic` object already. It does change the hypothetical enforcement bot, though.

>I think I'd be ok making Tablegen enforce adding a new 'tag' to each diagnostic that has its StableId. Not sure I like the name OldStableIds, but that is bikeshedding. You'd just have to do an initial population of that, and a script to name them.... something would be acceptable.

If the initial StableId tag is just auto-generated from the existing definition of the diagnostic, is there any point in actually adding all those initial StableId tags to the `.td` files? Even if we add these, we'd still need some kind of PR-time (not build-time, see below) enforcement to spot devs renaming/splitting/merging diagnostics and remind them to update OldStableIds. Having all StableIds be explicit might make it more likely for a dev who renames a diagnostic to keep its stable ID the same, but they'd still probably need the PR bot to remind them to update OldStableIds.

>I think this approach is a little subtle. And unless we have a way of enforcing when this happened, it'll be missed. IMO, we should just bite the bullet, require StableId via tablegen, and just come up iwth a heuristic for the 'initial set' of errors+ warnings.
> ...
>I'd rather tablegen enforce the existence, a CI job/etc to do it would be acceptable.

Tblgen can only enforce rules that only depend on the current version of the `.td` files. For example, we can and should enforce a "no duplicate StableIds" rule in tblgen at build time. The PR bot would be needed to catch cases where someone renamed a diagnostic and didn't keep the StableId (implicit or explicit) stable. It would also be needed to catch the more complex "merged two diagnostics" or "split a diagnostic" cases. Those cases require looking at the `.td` files before and after the change.


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