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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits