[Bug c++/102775] New: Error recovery in the C++ parser should use fix-it hints

[Bug c++/102775] New: Error recovery in the C++ parser should use fix-it hints

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102775

            Bug ID: 102775
           Summary: Error recovery in the C++ parser should use fix-it
                    hints
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: enhancement
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
                CC: dmalcolm at gcc dot gnu.org
            Blocks: 16663
  Target Milestone: ---

The scope of this is probably big and I have no idea how feasible it is, but
...


Consider the example in PR 102771:

template<typename.. T> void f(T...) { }

And the errors:

vart.C:1:18: error: expected nested-name-specifier before ‘.’ token
    1 | template<typename.. T> void f(T...) { }
      |                  ^
vart.C:1:18: error: expected ‘>’ before ‘.’ token
vart.C:1:29: error: variable or field ‘f’ declared void
    1 | template<typename.. T> void f(T...) { }
      |                             ^
vart.C:1:31: error: ‘T’ was not declared in this scope
    1 | template<typename.. T> void f(T...) { }
      |                               ^


After the first two errors g++ is just confused, and so we get the unhelpful
"declared void" error (PR 102774).

We should add a fix-it for the '...' token (PR 102771), and then it would be
awesome if the parser acted as though that was what it had seen. Replace the
two bogus '.' tokens with '...' and carry on. The rest of the function would
parse OK! We would not get the "declared void" error and we would not get the
"'T' was not declared in this scope" error.

For the example in PR 16663 comment 9, if we assumed that the undeclared
"misspelled" really was intended to be the "mispelled" type that is in scope,
the rest of the function declaration would parse OK! We would not print the
next three errors.

For the example in PR 102773, if we had fix-its for those errors and pretended
that's what the code had said, we would only print 3 errors not 10.

Generally, our error recovery is often poor. It's helpful to try and keep going
after an error, so that code with multiple problems will print multiple errors,
and the user can fix them all before compiling again. This shortens the
edit-compile-debug cycle, and is good. But when we print four errors for a
single function declaration, because of a single misplaced character, that's
not good.

Any time we offer a fix-it it's likely we have made an educated guess about
what the code really meant. We should make the parser continue as though that
was actually what the code said. Obviously this won't always be correct, but in
my experience our fix-it hints are correct far more often than they're wrong.
Maybe sometimes it would make the diagnostics worse, but the "declared void"
cases are already bad anyway (although PR 102774 would help there).

We show great fix-it hints to users, we should use them as feedback to the
parser too.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16663
[Bug 16663] Poor parse error recovery with mispelled type in function
declaration

[Bug c++/102775] Error recovery in the C++ parser should use fix-it hints

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102775

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
P.S. this would have to done on a case-by-case basis, i.e. for each fix-it that
the compiler currently supports, decide how confident we are in that being
correct most of the time, and decide whether to use that specific fix-it to
help the parser recover. So it wouldn't need to be done all at once for every
fix-it.

[Bug c++/102775] Error recovery in the C++ parser should use fix-it hints

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102775

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Here's another example:

#include <limits>
int i = numeric_limits<double>::digits10;

lim.cc:2:9: error: 'numeric_limits' was not declared in this scope; did you
mean 'std::numeric_limits'?
    2 | int i = numeric_limits<double>::digits10;
      |         ^~~~~~~~~~~~~~
      |         std::numeric_limits
In file included from lim.cc:1:
/home/jwakely/gcc/14/include/c++/14.0.0/limits:312:12: note:
'std::numeric_limits' declared here
  312 |     struct numeric_limits : public __numeric_limits_base
      |            ^~~~~~~~~~~~~~
lim.cc:2:24: error: expected primary-expression before 'double'
    2 | int i = numeric_limits<double>::digits10;
      |                        ^~~~~~

The "expected primary-expression before 'double'" part is just noise. There's
only one problem in the code, and if you add the suggested "std::"
qualification then the code is valid.

If we treated "numeric_limits" as an alias for "std::numeric_limits" after the
first error, then the rest of the file would compile successfully. We'd still
give an error and the code would be ill-formed, but we'd only complain about
the one location that actually has a bug.

Presumably to make this work we would need to make the result of the "did you
mean" lookup available to the compiler (not just to the diagnostic machinery).
If there's a match, like std::numeric_limits in this case, then do name lookup
for that and if it's found and is a type then act as though we'd seen a
using-declaration/typedef for that name to make it available as just
"numeric_limits".

For a name where the match is the same name but with a namespace qualification,
a using-declaration would suffice to make it available with the unqualified
name. For a misspelled name where the match is spelled differently, we could
introduce a typedef (for types) or reference (for objects and functions) to
make the name usable with the misspelled name.

Sometimes doing this might make the code even more invalid and produce
additional diagnostics that wouldn't be present today. I think it would need to
be tried and tested on a variety of code to see whether it's usually an
improvement or makes things worse. I suspect that for the majority of code with
a single mistake (like using a name unqualified) it would help.

Introducing these aliases for unqualified/misspelled names could potentially
also change the meaning of the following code to make it something different
from what was really intended, e.g. a fix-it for "foo()" suggests "Foo()" but
the user actually meant to call "food()". The code has already hit an error and
so we'll never generate wrong code, so this can only result in additional
spurious diagnostics that wouldn't be emitted today.