erichkeane 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());
}
----------------
aaron.ballman wrote:
> 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.
I'd suggest more of a 'Make sure other side is not this side' :D
================
Comment at: clang/include/clang/Sema/ParsedAttr.h:813
void takeAllFrom(AttributePool &pool) {
takePool(pool);
pool.Attrs.clear();
----------------
so: `assert(&pool != this && "Stealing from yourself? Super bad...")`.
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
