[cfe-dev] Clarifying the roles and requirements on fixit hints in diagnostics

[Chandler Carruth]

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:
---- x.cc ----
class X;
struct X {
  X() {}
};
X x;
----
This code is well formed, but the warning will suggest replacing 'struct'
with 'class'. Doing so makes the code ill-formed. If we recover as-if the
fixit hint were applied, we would error on this code when the warning is
turned on, which seems rather surprising. ;] If we don't recover as if the
fixit hint were applied, and run Clang with -fixit, we accept the code, but
then alter the code to a text sequence which if we recompile is rejected.
Again, rather surprising. Currently, Clang's warnings which have fixit hints
attached have a mixture of these policies, but more often follow the latter
policy of no recovery. For some, this is a moot point -- the code parses the
same either way and the hint merely removes redundant text or adds
clarifying text. The question is what *should* the rest of the warnings with
fixit hints do?

One option would be to require that warnings with direct fixit hints *can*
recover as if the hint were applied, but provide a hook so that the recovery
is only performed when that warning is promoted to an error or when running
with -fixit in effect.

A second option is to require that recovery not be performed for warning
fixit hints and that -fixit not apply them automatically. Essentially treat
them the same as note fixit hints.

I prefer the second option as it is simpler, and I think provides a better
user experience. There are several warnings with a note attached to them
purely to provide a fixit hint without recover or automatic application. The
output would be simpler and more clear if these could be directly attached
to the warnings.

Thoughts? Once this is a bit more clear, I'll start fixing documentation.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110410/7d87328d/attachment.html>