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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Sema/DeclSpec.h:2516-2521
+  void takeAttributes(ParsedAttributes &attrs) {
     Attrs.takeAllFrom(attrs);
 
-    if (!lastLoc.isInvalid())
-      SetRangeEnd(lastLoc);
+    if (attrs.Range.getEnd().isValid())
+      SetRangeEnd(attrs.Range.getEnd());
   }
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > 
> > > I blindly changed this and it took me a while to figure out that's wrong from the test failures: 
> > > 
> > > `Attrs.takeAllFrom(Attrs)`...
> > Oh god, I'm so sorry for that terrible suggestion, I hadn't spotted I was reusing the name. Feel free to go with `A` or `PA` or something for the parameter name to avoid that conflict.
> Haha, no problem. Do you think adding an assertion for this case to `takeAllFrom()` (and `takeOneFrom()`) makes sense?
An assertion that the attributes are actually taken from the argument (so validating the size of the container after taking from it)? Probably wouldn't hurt.


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

https://reviews.llvm.org/D121201