hans added a comment.
Looking good overall, just a few nits.
Out of curiosity, where would one run into this? Does MS use this in any
library headers or such?
================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3314
if (getLangOpts().CPlusPlus && Tok.is(tok::identifier)) {
- VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
- assert((Specifier == VirtSpecifiers::VS_Final ||
- Specifier == VirtSpecifiers::VS_GNU_Final ||
- Specifier == VirtSpecifiers::VS_Sealed) &&
+ for (VirtSpecifiers::Specifier Specifier = isCXX11VirtSpecifier(Tok);
+ Specifier != VirtSpecifiers::VS_None;
----------------
This 'for' construct is hard to follow. I think it might be easier to follow as
a while loop, maybe
```
while ((Specifier = isCXX11VirtSpecifier(Tok)) != VirtSpecifiers::VS_None) {
```
Also the "is" prefix in isCXX11VirtSpecifier is confusing given that it doesn't
return a bool.
================
Comment at: clang/lib/Sema/DeclSpec.cpp:1479
+ case VS_Abstract:
+ VS_abstractLoc = Loc;
+ break;
----------------
nit: the other cases just use one line
================
Comment at: clang/lib/Sema/DeclSpec.cpp:1493
case VS_Sealed: return "sealed";
+ case VS_Abstract:
+ return "abstract";
----------------
nit: skip the newline for consistency here too
================
Comment at: clang/lib/Sema/SemaDecl.cpp:16449
- if (FinalLoc.isValid())
+ if (IsAbstract) {
+ Record->markAbstract();
----------------
nit: it would be more common in clang to omit the braces
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102517/new/
https://reviews.llvm.org/D102517
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
