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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder marked 7 inline comments as done.
tbaeder added inline comments.


================
Comment at: clang/include/clang/Sema/ParsedAttr.h:1105
   void clearListOnly() {
     ParsedAttributesView::clearListOnly();
     Range = SourceRange();
----------------
aaron.ballman wrote:
> erichkeane wrote:
> > This is... oh boy.  I'm hopeful you can remove this type as well.
> +1, it'd be fantastic if we could, otherwise we're storing the range twice for this type (and it's named `Range` both times).
With the range in `ParsedAttributesView`, this is indeed not needed.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:500
 /// a C++11 attribute in "gnu" namespace.
-void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
-                                   SourceLocation AttrNameLoc,
-                                   ParsedAttributes &Attrs,
-                                   SourceLocation *EndLoc,
-                                   IdentifierInfo *ScopeName,
-                                   SourceLocation ScopeLoc,
-                                   ParsedAttr::Syntax Syntax,
-                                   Declarator *D) {
+void Parser::ParseGNUAttributeArgs(
+    IdentifierInfo *AttrName, SourceLocation AttrNameLoc,
----------------
aaron.ballman wrote:
> It looks like some unrelated formatting changes snuck in? I'm not opposed, but it's a bit easier to review if this was an NFC commit (before or after landing this patch) because it makes for a smaller diff here. (I noticed this in a few spots, only commenting about it here.)
I've tried reverting some of them, but `git clang-format` adds them back every time...


================
Comment at: clang/lib/Parse/ParseDecl.cpp:1424
+    IdentifierInfo &ObjCBridgeRelated, SourceLocation ObjCBridgeRelatedLoc,
+    ParsedAttributes &attrs, SourceLocation *endLoc, IdentifierInfo *ScopeName,
+    SourceLocation ScopeLoc, ParsedAttr::Syntax Syntax) {
----------------
erichkeane wrote:
> Could we lose the endLoc here as well?
Possibly. There are still a few of the attribute parsing functions left that take an endLoc (esp. when it comes to attribute args). I can look into that after this patch, it's big enouhg as it is I think.


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

https://reviews.llvm.org/D121201