[PATCH] D121201: [clang] Merge the SourceRange into ParsedAttributes

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Just a couple of questions/comments.  Otherwise this seems pretty ok.



================
Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:745
 
-      ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, &endLoc,
+      ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs, nullptr,
                             nullptr, SourceLocation(), ParsedAttr::AS_GNU,
----------------
This change is concerning, shouldn't ParseGNUAttributeArgs just be losing its 'EndLoc' pointer instead?


================
Comment at: clang/lib/Parse/ParseDecl.cpp:1424
+    IdentifierInfo &ObjCBridgeRelated, SourceLocation ObjCBridgeRelatedLoc,
+    ParsedAttributes &attrs, SourceLocation *endLoc, IdentifierInfo *ScopeName,
+    SourceLocation ScopeLoc, ParsedAttr::Syntax Syntax) {
----------------
Could we lose the endLoc here as well?


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3207
+  if (Tok.is(tok::kw___attribute)) {
+    ParsedAttributes Attrs(AttrFactory);
     MaybeParseGNUAttributes(Attrs);
----------------
This seems like a particularly strange change, What is the reasoning for this?  Is it just that the Attrs are unused?


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:4361
 
-/// ParseCXX11AttributeSpecifier - Parse a C++11 or C2x attribute-specifier.
+/// ParseCXX11AttributeSpecifierInternal - Parse a C++11 or C2x
+/// attribute-specifier.
----------------
I believe we aren't supposed to be putting the 'name' of the function in the comments anymore, can you just remove it instead of updating it?


================
Comment at: clang/lib/Sema/SemaType.cpp:8129
+
+  state.setParsedNoDeref(false);
+  if (attrs.empty())
----------------
What is the justification for this set of changes?  Is it simply to 'early exit' rather than make the unnecessary copy?


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

https://reviews.llvm.org/D121201