svenvh added inline comments.

================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:969
+
+void OpenCLBuiltinTestEmitter::getTypeLists(
+    Record *Type, TypeFlags &Flags, std::vector<Record *> &TypeList,
----------------
Anastasia wrote:
> I would add a documentation for this function, it is not very obvious.
Not sure what documentation you are expecting?  Are there any particular lines 
that are "not very obvious"?
Note that there is already a comment for this method up in the class 
declaration.


================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:1004
+    SmallVectorImpl<SmallVector<std::string, 2>> &Types) {
+  // Find out if there are any GenTypes in this signature, and if so, calculate
+  // into how many signatures they will expand.
----------------
Anastasia wrote:
> Maybe this should be lifted as documentation of the header of this function?
This comment only applies to the first loop-nest.  It documents an 
implementation detail, so I don't think this comment should be lifted to the 
method's declaration comment.


================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:1033
+    for (unsigned ArgNum = 0; ArgNum < Signature.size(); ArgNum++) {
+      // For differently-sized GenTypes in a parameter list, the smaller
+      // GenTypes just repeat.
----------------
Anastasia wrote:
> I don't get this comment and its exact reference to the code.
I have expanded the comment, and also factored out an index into a variable.  
Hopefully that makes it clear?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97869/new/

https://reviews.llvm.org/D97869

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to