jlebar marked 2 inline comments as done.
jlebar added a comment.

I'm going to submit this and send a patch to reuse FunctionDeclAndLoc.  But I'm 
happy to add a comment about the note as well.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6707
 
+def note_called_by : Note<"called by %0">;
 def err_kern_type_not_void_return : Error<
----------------
rnk wrote:
> Do you think it's worth trying to indicate why the root of the "called by" 
> notes must be emitted? I'm not suggesting we do it in this patch, just 
> wondering.
It seems just like e.g. note_template_class_instantiation_here, so I am not 
entirely sure what is the point of confusion.  But if you can clarify that, I 
am happy to add a comment in a separate patch -- this stuff is already quite 
complicated, so I'm in favor of whatever we can do to make it clearer.


================
Comment at: clang/include/clang/Sema/Sema.h:9259
   /// use this to avoid emitting the same deferred diag twice.
-  llvm::DenseSet<std::pair<FunctionDecl *, unsigned>> LocsWithCUDACallDiags;
+  llvm::DenseSet<std::pair<CanonicalDeclPtr<FunctionDecl>, unsigned>>
+      LocsWithCUDACallDiags;
----------------
rnk wrote:
> So, part of me wants to use FunctionDeclAndLoc here instead of std::pair, but 
> then you'd have to bring back all the hashing machinery instead of getting it 
> for free. Up to you, I guess.
Oh, and now make its hash function depend on both members instead of just the 
FD?

I actually like that change, but let me make it in a separate patch.


================
Comment at: clang/include/clang/Sema/Sema.h:9274
+  llvm::DenseMap</* Callee = */ CanonicalDeclPtr<FunctionDecl>,
+                 /* Caller = */ FunctionDeclAndLoc>
+      CUDAKnownEmittedFns;
----------------
rnk wrote:
> I guess there can be many callers, but we always record the first one that 
> caused this function to be emitted.
Yes, exactly.

I was briefly worried about cycles here, but I think we're OK because 
ultimately we need to end up at an a priori known-emitted function, and that's 
a root in this tree.


https://reviews.llvm.org/D25704



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

Reply via email to