rjmccall accepted this revision. rjmccall added inline comments.
================ Comment at: include/clang/AST/Decl.h:901 + /// member functions. + unsigned ImplicitParamKind : 3; }; ---------------- aaron.ballman wrote: > 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.) Yeah, I agree that it's unfortunate that we have two separate classes for parameter declarations. https://reviews.llvm.org/D33735 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
