[PATCH] D25809: [CUDA] Improved target attribute-based overloading.

[Artem Belevich via cfe-commits]

tra added inline comments.


================
Comment at: lib/Sema/SemaCUDA.cpp:87
+
+  if ((HasHostAttr && HasDeviceAttr) || ForceCUDAHostDeviceDepth > 0)
+    return CFT_HostDevice;
----------------
jlebar wrote:
> Checking ForceCUDAHostDeviceDepth here is...yeah.  Especially because the other overload of this function doesn't do it.
> 
> I know you get rid of it in the next patch, but how much work would it be to get rid of it here?  It definitely makes this patch harder to check.
OK. Moved the check to the place where I use it.


================
Comment at: lib/Sema/SemaCUDA.cpp:791
+  CUDAFunctionTarget NewTarget = IdentifyCUDATarget(NewFD);
+  for (auto OldND : Previous) {
+    FunctionDecl *OldFD = OldND->getAsFunction();
----------------
jlebar wrote:
> If this is just a Decl* or NamedDecl*, can we write out the type?
I'm not sure what exactly you'd like to see. Diags will print out the line and target info for both sides.
Could you give me example of existing diagnostics that would be similar to what you want?


================
Comment at: lib/Sema/SemaCUDA.cpp:793
+    FunctionDecl *OldFD = OldND->getAsFunction();
+    if (!OldFD || OldFD->isFunctionTemplateSpecialization())
+      continue;
----------------
jlebar wrote:
> Please add a comment explaining why we ignore template specializations.
That's another check that's no longer needed as the only template instantiations/specializations that end up in the overload set are those that were instantiated from templates with matching target.



================
Comment at: lib/Sema/SemaTemplate.cpp:8117
+    if (LangOpts.CUDA &&
+        IdentifyCUDATarget(Specialization) != IdentifyCUDATarget(Attr)) {
+      FailedCandidates.addCandidate().set(
----------------
jlebar wrote:
> Can we just pass D here (and thus not write the new overload of IdentifyCUDATarget)?
Nope. D is a Declarator which is very different form FunctionDecl.
It carries info about what we've parsed, but we didn't construct a FunctionDecl from it yet.


https://reviews.llvm.org/D25809