[PATCH] D111883: [Parse] Improve diagnostic and recoveryy when there is an extra override in the outline method definition.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:948
+def err_virt_specifier_outside_class : Error<
+  "'%0' virt-specifier is not allowed outside a class definition">;
+
----------------
sammccall wrote:
> sammccall wrote:
> > `virt-specifier` is standardese. I think dropping virt, i.e. `'override' specifier...` is clear enough.
> Hm, is "outside a class definition" the clearest way to explain the mistake?
> 
> I guess this is handling two classes of problem:
> - repeating `override` on an out-of-line definition (motivating case)
> - placing `override` on a free function
> 
> I'd slightly prefer the motivating case to mention "out-of-line", but that wording doesn't fit the other case. I'm not sure whether it's worth trying to split them 
yeah, the first one is the motivating case, I think it is the most common one, but I'm tempted to keep it as-is rather than creating a separate diagnostic message for a less-common case.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:2039
     // start of a function definition in GCC-extended K&R C.
     if (!isDeclarationAfterDeclarator()) {
 
----------------
sammccall wrote:
> Do we too have to consider K&R?
> 
> ```
> typedef int override;
> void foo(a) 
>     override a; 
> {
> }
> ```
> 
> I guess not because this behavior only fires for c++?
right, the change only affects C++ code -- isCXX11VirtSpecifier returns null for non-C++11-or-later code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111883/new/

https://reviews.llvm.org/D111883