dbartol wrote: @erichkeane >> _should all errors/warnings have explicit stable IDs?_ > >Yes, there IS. This removes an entire class of 'rename problems' (AND the most >likely ones!) to the point that PR-time enforcement will catch >orders-of-magnitude fewer things. OF your list above, 96 + 15 + 240 all seem >like 'not a problem'. This would enforce that breaking the SARIF name is an >actual, active, intentional thing, which would remove all of the 13. I would >still want periodic checking for these, but it would remove our risk of >breaking them by almost 100%.
OK, let's do that. A couple details: First, what's the best way to ensure that a developer changing or adding a diagnostic understands that the stable ID needs to be stable? Using the `StableId<>` class from my tblgen PR makes it more obvious, rather than just adding a second unnamed parameter to the `Warning<>` and `Error<>` classes. Does that sound OK? Second, can I add those explicit stable IDs in a follow-up PR? So the PR sequence would be something like: 1. The current PR (implicit stable IDs) 2. Add tblgen support for (optional) explicit stable IDs [PR here](https://github.com/dbartol/llvm-project/pull/2) 3. Add explicit stable IDs for all warnings and errors. 4. Make explicit stable IDs required for warnings and errors. I'm kind of dreading when PRs 3 and 4 hit my (or anyone else's) downstream fork, but I think having them as separate PRs will make that easier. It would be possible to merge PR 3 (which will probably have some merge conflicts), then go add stable IDs to all the downstream-only diagnostics, then merge PR 4. https://github.com/llvm/llvm-project/pull/168153 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
