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:<div><br></div><div><div>% cat x.cc</div>
<div>struct S {</div><div>  X() : a(1), b(2) {}</div><div>  explicit X(int) : a(1), b(2) {}</div><div><br></div><div>  int a, b;</div><div>};</div><div><br></div><div><div>% ./bin/clang x.cc                                    </div>
<div>x.cc:2:7: error: unexpected ':' in (non-constructor) member function declaration</div><div>  X() : a(1), b(2) {}</div><div>      ^</div><div>      ;</div><div>x.cc:2:11: error: expected parameter declarator</div>
<div>  X() : a(1), b(2) {}</div><div>          ^</div><div>x.cc:2:11: error: expected ')'</div><div>x.cc:2:10: note: to match this '('</div><div>  X() : a(1), b(2) {}</div><div>         ^</div><div>x.cc:2:9: error: C++ requires a type specifier for all declarations</div>
<div>  X() : a(1), b(2) {}</div><div>        ^</div><div>x.cc:2:17: error: expected parameter declarator</div><div>  X() : a(1), b(2) {}</div><div>                ^</div><div>x.cc:2:17: error: expected ')'</div><div>
x.cc:2:16: note: to match this '('</div><div>  X() : a(1), b(2) {}</div><div>               ^</div><div>x.cc:2:20: error: generalized initializer lists are a C++0x extension unsupported in Clang</div><div>  X() : a(1), b(2) {}</div>
<div>                   ^</div><div>x.cc:2:15: error: C++ requires a type specifier for all declarations</div><div>  X() : a(1), b(2) {}</div><div>        ~     ^</div><div>x.cc:3:29: error: expected parameter declarator</div>
<div>  explicit X(int) : a(1), b(2) {}</div><div>                            ^</div><div>x.cc:3:29: error: expected ')'</div><div>x.cc:3:28: note: to match this '('</div><div>  explicit X(int) : a(1), b(2) {}</div>
<div>                           ^</div><div>x.cc:3:32: error: generalized initializer lists are a C++0x extension unsupported in Clang</div><div>  explicit X(int) : a(1), b(2) {}</div><div>                               ^</div>
<div>x.cc:3:27: error: C++ requires a type specifier for all declarations</div><div>  explicit X(int) : a(1), b(2) {}</div><div>                          ^</div><div>x.cc:5:10: error: C++ requires a type specifier for all declarations</div>
<div>  int a, b;</div><div>         ^</div><div>x.cc:5:10: error: duplicate member 'b'</div><div>x.cc:3:27: note: previous declaration is here</div><div>  explicit X(int) : a(1), b(2) {}</div><div>                          ^</div>
<div>14 errors generated.</div></div><div><br></div><div><br></div><div>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.</div>
<div><br></div><div>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:</div><div><br></div><div>"invalid ':' following a non-constructor member declaration, did you mean ';'?"</div>
<div><br></div>If it might be an initializer list, I would suggest:</div><div><br></div><div>"':' implies a constructor initializer, but 'foo' is not a constructor"</div><div><br></div><div>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).</div>
<div><br><div class="gmail_quote">On Fri, May 27, 2011 at 5:04 PM, Mihai Rusu <span dir="ltr"><<a href="mailto:dizzy@google.com">dizzy@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi,<br>
<br>
This is my first patch sent directly to clang upstream, please be gentle :)<br>
<br>
"git show" generated patch, apply with -p1 in .../tools/clang. Passes<br>
"make clang-test" (I'm using cmake builds).<br>
<br>
When trying the same mistype with namespace scope declarations we have<br>
a somewhat better situation, the first diagnostic is confusing the the<br>
next one is pretty clear. I could try to address this case too if it<br>
seems valuable.<br>
<br>
$ cat case-7873.cc<br>
class Class {<br>
  int m;<br>
  Class();<br>
  virtual ~Class():<br>
  virtual int Func();<br>
};<br>
<br>
int Class::Func():<br>
Class::Class():m(10){}<br>
<br>
$ ./bin/clang++ -fsyntax-only case-7873.cc<br>
# Generated by the patch.<br>
case-7873.cc:4:19: error: unexpected ':' in (non-constructor) member function<br>
      declaration<br>
  virtual ~Class():<br>
                  ^<br>
                  ;<br>
<br>
# Current diagnostic for namespace scope mistyped declarations.<br>
case-7873.cc:9:15: error: expected '{' or ','<br>
Class::Class():m(10){}<br>
              ^<br>
case-7873.cc:8:18: error: only constructors take base initializers<br>
int Class::Func():<br>
                 ^<br>
3 errors generated.<br>
<br>
--<br>
<font color="#888888">Mihai Rusu<br>
</font><br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div>