aaron.ballman added a comment.
Thank you for working on this, I know it was a slog! I think this is pretty
close to ready, though you should definitely add a release note about the
changes.
================
Comment at: clang/include/clang/Parse/Parser.h:1605-1606
// C99 6.9: External Definitions.
DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &Attrs,
+ ParsedAttributes &DeclSpecAttrs,
ParsingDeclSpec *DS = nullptr);
----------------
Should we rename `Attrs` to `DeclaratorAttrs` or something along those lines,
so it's easier for someone to distinguish which attributes go where? (Same
suggestion applies throughout the review.)
================
Comment at: clang/lib/Parse/ParseObjc.cpp:67-69
+ for (const auto &PA : Attrs)
+ if (PA.isGNUAttribute())
+ Diag(Tok.getLocation(), diag::err_objc_unexpected_attr);
----------------
llvm::for_each?
================
Comment at: clang/lib/Parse/ParseOpenMP.cpp:1997
+ ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
+
----------------
We can sink this down to the only place it's used in the function, right?
================
Comment at: clang/lib/Parse/Parser.cpp:735-737
+ ParsedAttributes DeclSpecAttrs(AttrFactory);
+ while (MaybeParseCXX11Attributes(attrs) ||
+ MaybeParseGNUAttributes(DeclSpecAttrs))
----------------
We probably could use some comments here explaining why we parse some into one
container and others into another container.
================
Comment at: clang/lib/Parse/Parser.cpp:1096-1097
+ ParsingDeclSpec &DS, AccessSpecifier AS) {
+ DS.SetRangeStart(DeclSpecAttrs.Range.getBegin());
+ DS.SetRangeEnd(DeclSpecAttrs.Range.getEnd());
+ DS.takeAttributesFrom(DeclSpecAttrs);
----------------
How sure are you that this isn't overwriting existing range data that's
potentially relevant? e.g., where declaration specifiers and attributes are
mixed together such that the attribute range doesn't cover all of the
declaration specifiers?
================
Comment at: clang/test/Parser/attr-order.cpp:20
__attribute__((cdecl)) __declspec(dllexport) [[noreturn]] void f(); //
expected-error {{an attribute list cannot appear here}}
-__attribute__((cdecl)) [[noreturn]] __declspec(dllexport) void g(); //
expected-error {{an attribute list cannot appear here}}
+__attribute__((cdecl)) [[noreturn]] __declspec(dllexport) void g();
----------------
This makes me happy!!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137979/new/
https://reviews.llvm.org/D137979
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
