rjmccall added inline comments.
================ Comment at: include/clang/AST/Decl.h:1387 + IPK_CapturedContext, /// Parameter for captured context + IPK_GeneralParam, /// General implicit parameter + }; ---------------- ABataev wrote: > rjmccall wrote: > > I would just call this "Other" and document it as being for kinds of > > implicit parameters that we haven't seen a purpose in categorizing yet. > > (Or you could just classify them all, I suppose.) > > > > We can use C++11 features in Clang now, so I would recommend hoisting this > > type out of ImplicitParamDecl and making it an enum class. You can then > > drop the "IPK_" prefixes. > It's hard to classify them, in most cases they just represent some general > parameters used for correct codegen of function types and that's it. That's fair. ================ Comment at: include/clang/AST/Decl.h:901 + /// member functions. + unsigned ImplicitParamKind : 3; }; ---------------- 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. https://reviews.llvm.org/D33735 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits