[cfe-dev] [PATCH] Improve diagnostic mistyping ":" for "; " after member function declarations (PR7873)

[Chandler Carruth]

This warning is really nice in the case it handles, but it does terrible
things when the mistake is an incorrect constructor name instead of a typo
of ';'. Consider the following code:

% cat x.cc
struct S {
  X() : a(1), b(2) {}
  explicit X(int) : a(1), b(2) {}

  int a, b;
};

% ./bin/clang x.cc
x.cc:2:7: error: unexpected ':' in (non-constructor) member function
declaration
  X() : a(1), b(2) {}
      ^
      ;
x.cc:2:11: error: expected parameter declarator
  X() : a(1), b(2) {}
          ^
x.cc:2:11: error: expected ')'
x.cc:2:10: note: to match this '('
  X() : a(1), b(2) {}
         ^
x.cc:2:9: error: C++ requires a type specifier for all declarations
  X() : a(1), b(2) {}
        ^
x.cc:2:17: error: expected parameter declarator
  X() : a(1), b(2) {}
                ^
x.cc:2:17: error: expected ')'
x.cc:2:16: note: to match this '('
  X() : a(1), b(2) {}
               ^
x.cc:2:20: error: generalized initializer lists are a C++0x extension
unsupported in Clang
  X() : a(1), b(2) {}
                   ^
x.cc:2:15: error: C++ requires a type specifier for all declarations
  X() : a(1), b(2) {}
        ~     ^
x.cc:3:29: error: expected parameter declarator
  explicit X(int) : a(1), b(2) {}
                            ^
x.cc:3:29: error: expected ')'
x.cc:3:28: note: to match this '('
  explicit X(int) : a(1), b(2) {}
                           ^
x.cc:3:32: error: generalized initializer lists are a C++0x extension
unsupported in Clang
  explicit X(int) : a(1), b(2) {}
                               ^
x.cc:3:27: error: C++ requires a type specifier for all declarations
  explicit X(int) : a(1), b(2) {}
                          ^
x.cc:5:10: error: C++ requires a type specifier for all declarations
  int a, b;
         ^
x.cc:5:10: error: duplicate member 'b'
x.cc:3:27: note: previous declaration is here
  explicit X(int) : a(1), b(2) {}
                          ^
14 errors generated.


I think you need to look at the tokens following the ':' and try to
determine whether it is followed by an initializer list or a declaration. We
already have some logic for this in the parser in order to diagnose a
missing ';' after 'struct S {}'. It's likely re-usable here.

Also, I'd change the text of the message. If you are confident that it is
followed by a declaration, and thus should be recovered as a ';' maybe:

"invalid ':' following a non-constructor member declaration, did you mean
';'?"

If it might be an initializer list, I would suggest:

"':' implies a constructor initializer, but 'foo' is not a constructor"

I'm not convinced we should issue a FixIt hint for the latter case to
correct the name to the constructor. Maybe compute the edit distance and
look to see if there are any non-constructor aspects to the declaration?
(IE, fixit hint to fix the name if the edit distance is below N, there is no
return type or specifiers which aren't allowed on a constructor).

On Fri, May 27, 2011 at 5:04 PM, Mihai Rusu <dizzy at google.com> wrote:

> Hi,
>
> This is my first patch sent directly to clang upstream, please be gentle :)
>
> "git show" generated patch, apply with -p1 in .../tools/clang. Passes
> "make clang-test" (I'm using cmake builds).
>
> When trying the same mistype with namespace scope declarations we have
> a somewhat better situation, the first diagnostic is confusing the the
> next one is pretty clear. I could try to address this case too if it
> seems valuable.
>
> $ cat case-7873.cc
> class Class {
>  int m;
>  Class();
>  virtual ~Class():
>  virtual int Func();
> };
>
> int Class::Func():
> Class::Class():m(10){}
>
> $ ./bin/clang++ -fsyntax-only case-7873.cc
> # Generated by the patch.
> case-7873.cc:4:19: error: unexpected ':' in (non-constructor) member
> function
>      declaration
>  virtual ~Class():
>                  ^
>                  ;
>
> # Current diagnostic for namespace scope mistyped declarations.
> case-7873.cc:9:15: error: expected '{' or ','
> Class::Class():m(10){}
>              ^
> case-7873.cc:8:18: error: only constructors take base initializers
> int Class::Func():
>                 ^
> 3 errors generated.
>
> --
> Mihai Rusu
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110614/b31185e2/attachment.html>