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

Reply via email to