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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

This seems very nice to me, I'd hit the bad diagnostic before but hadn't noticed the bad recovery.
AFAICT this can't break any valid code.



================
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">;
+
----------------
`virt-specifier` is standardese. I think dropping virt, i.e. `'override' specifier...` is clear enough.


================
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:
> `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 


================
Comment at: clang/lib/Parse/ParseDecl.cpp:2039
     // start of a function definition in GCC-extended K&R C.
     if (!isDeclarationAfterDeclarator()) {
 
----------------
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++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111883