aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Aside from one minor nit, LGTM!
================
Comment at: include/clang/AST/Decl.h:901
+ /// member functions.
+ unsigned ImplicitParamKind : 3;
};
----------------
rjmccall wrote:
> ABataev wrote:
> > aaron.ballman wrote:
> > > ABataev wrote:
> > > > aaron.ballman wrote:
> > > > > It's a bit strange to me that the non-parameter declaration bits now
> > > > > have a field for implicit parameter information. Why here instead of
> > > > > `ParmVarDeclBits`?
> > > > Actually, `ImplicitParamDecl` already uses some bits from the
> > > > `NonParmVarDeclBitfields`, at least it may be marked as
> > > > `ARCPseudoStrong` for ObjC. That's why I had to reuse
> > > > `NonParmVarDeclBitfields` part.
> > > Ew. That's nasty and we should probably fix that (not as part of this
> > > patch). Can you add a FIXME here?
> > Ok, I will add FIXME for `ARCPseudoStrong` and for the `ImplicitParamKind`
> > bitfields
> The FIXME doesn't make sense because ImplicitParamDecl is not a subclass of
> ParamVarDecl.
>
> The comment here needs to be updated.
> The FIXME doesn't make sense because ImplicitParamDecl is not a subclass of
> ParamVarDecl.
Then the class name is poor, because I would not expect to find *anything*
about parameter declarations (implicit or otherwise) in a class named
`NonParmVarDeclBitfields`. However, I see the logic behind the split better
now, so thank you for that. (Nothing needs to be done here for this patch.)
================
Comment at: lib/Serialization/ASTWriterDecl.cpp:918
Record.push_back(D->isPreviousDeclInSameBlockScope());
+ if (auto *IPD = dyn_cast<ImplicitParamDecl>(D))
+ Record.push_back(static_cast<unsigned>(IPD->getParameterKind()));
----------------
aaron.ballman wrote:
> `const auto *`
This is still missing the `const` qualifier.
https://reviews.llvm.org/D33735
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
