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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to