Clarifying the roles and requirements on fixit hints in diagnostics

I know I have been confused on more than one occasion about the precise requirements for issuing a fixit hint. After several IRC conversations (thanks to those that participated) I think I’ve got a decent understanding, and I’d like to add some sections to the Clang documentation which spell this out for future reference. A brief summary of my understanding lest I have gotten confused again:

  1. Fixit hints attached to an error diagnostic must recover the parse/semantic analysis/etc as if the fix had been applied. This won’t ever miscompile code as the compilation cannot succeed after issuing an error unless Clang has been asked to automatically apply these fixes, in which case the compile will succeed, and accurately reflect the (newly updated) code.

  2. Fixit hints on notes must not have any impact on the compilation. The also are not automatically applied to the code by the -fixit flag.

  3. There can be only one hint attached to a diagnostic, and thus if the hint is attached to the error or warning diagnostic it must be an extremely confident fix with no other viable candidates. When there are multiple viable candidate fixes, they should be presented as multiple fixit hint bearing notes.

The one area this doesn’t cover are fixit hints attached to warnings. These are trickier. Previously, it has been suggested that they should follow the same rules as those attached to errors, but that has some serious problems. Suppose this is the approach is used for -Wmismatched-tags:
---- ----
class X;
struct X {
X() {}
X x;

I like the second option with a refinement: the warning fixit must be one that removes this warning and does not change the semantics or validity of the program (it may change the AST). If it may change the program then it needs to be a note. Then, -fixit should apply warning

That way, we never do recovery that follows the fixits except on errors, the fixits like -Wparenthesis will still fire to add the parens in a way that doesn’t actually change anything, -Wmismatched-tags would use a note fixit to suggest its change.

Actually -Wmismatched-tags should also add public: / private: so that we could upgrade its fixit from a note to a warning and let clang -fixit fix things.

Making sure all the warnings choose the right level gives us a place for bugs to live, but I think it’s worth it for the ability to see “clang -fixit” fix the warnings it knows how to fix.