[Bug c++/105840] New: confusing diagnostic when naming the wrong class in a constructor

[Bug c++/105840] New: confusing diagnostic when naming the wrong class in a constructor

[barry.revzin at gmail dot com]

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

            Bug ID: 105840
           Summary: confusing diagnostic when naming the wrong class in a
                    constructor
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: barry.revzin at gmail dot com
  Target Milestone: ---

In this code:

template <typename T> struct X { };

struct A {
    A(int i);
    A(X<int> x);
};

struct B {
    A(int i);
    A(X<int> x);
};

I was copying the constructors from A into B, and forgot to change the name.
This is, indeed, ill-formed.

However, this is what gcc tells me:

<source>:9:7: error: expected unqualified-id before 'int'
    9 |     A(int i);
      |       ^~~
<source>:9:7: error: expected ')' before 'int'
    9 |     A(int i);
      |      ~^~~
      |       )
<source>:10:14: error: invalid declarator before 'x'
   10 |     A(X<int> x);
      |              ^
<source>:10:13: error: expected ')' before 'x'
   10 |     A(X<int> x);
      |      ~      ^~
      |             )

Clang does at least point me to the A, but otherwise provides a
differently-confusing diagnostic about the parentheses?

<source>:9:7: error: expected member name or ';' after declaration specifiers
    A(int i);
    ~ ^
<source>:9:7: error: expected ')'
<source>:9:6: note: to match this '('
    A(int i);
     ^
<source>:10:14: error: expected ')'
    A(X<int> x);
             ^
<source>:10:6: note: to match this '('
    A(X<int> x);
     ^
<source>:10:7: error: member 'X' cannot have template arguments
    A(X<int> x);
      ^~~~~~

[Bug c++/105840] confusing diagnostic when naming the wrong class in a constructor

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Keywords|                            |diagnostic
                 CC|                            |mpolacek at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-06-03

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Thanks for the report.  Do you have any ideas about what kind of wording would
be appropriate here?

[Bug c++/105840] confusing diagnostic when naming the wrong class in a constructor

[barry.revzin at gmail dot com]

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

--- Comment #2 from Barry Revzin <barry.revzin at gmail dot com> ---
I think something to this effect maybe?

<source>:9:7: error: attempting to declare constructor for unrelated class 'A';
did you mean to use 'B'?
    9 |     A(int i);
      |     ^~
      |     B

[Bug c++/105840] confusing diagnostic when naming the wrong class in a constructor

[mpolacek at gcc dot gnu.org]

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

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Ah, that sounds great to me.

[Bug c++/105840] confusing diagnostic when naming the wrong class in a constructor

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
For a destructor we do a bit better, but it could still be improved. 

struct A {
  ~A();
};

struct C {
  ~A();
};

105840.C:14:3: error: declaration of ‘~A’ as member of ‘C’
   14 |   ~A();
      |   ^

Yes, the formal reason the code is invalid is that ~A cannot be a member of C,
but the actual reason is that the author made a typo or forgot to change it
after copy&paste.

Maybe similar wording should be used for the constructor and destructor cases,
and a fix-it suggesting changing the "A" to "C" would be nice.

Clang gives a user-friendly message, but no fix-it:

105840.C:6:3: error: expected the class name after '~' to name the enclosing
class
  ~A();
  ^
1 error generated.



For a similar case where A hasn't been declared yet, we don't do so well:

struct Foo {
  ~Fo();
};

105840.C:2:6: error: expected class-name before ‘(’ token
    2 |   ~Fo();
      |      ^


This is just a typo, but because "Fo" hasn't been declared prior to this, we
give a very different diagnostic compared to the "~A" case. It would be nice if
this also used similar wording to the other examples, and also gave a fix-it.

Clang does quite well here:

105840.C:2:4: error: undeclared identifier 'Fo' in destructor name
  ~Fo();
   ^~
   Foo
1 error generated.


Although I don't see any reason why Clang gives such different wording for the
"undeclared identifier" and "expected [...] to name the enclosing class" cases.
In both cases the problem is a destructor with a name that isn't the enclosing
class. Whether the name has been declared or not is irrelevant. I think Barry's
suggestion in comment 2 (or something like it) would be good for destructors
too, whether or not the "unrelated class 'A'" has previously been declared.