[PATCH] D25845: [CUDA] Ignore implicit target attributes during function template instantiation.

[Justin Lebar via cfe-commits]

jlebar added inline comments.


================
Comment at: include/clang/Sema/Sema.h:9396
+  CUDAFunctionTarget IdentifyCUDATarget(const FunctionDecl *D,
+                                        bool IgnoreImplicitHDAttr = false);
   CUDAFunctionTarget IdentifyCUDATarget(const AttributeList *Attr);
----------------
We usually call them "target attributes", not "HD attributes"?


================
Comment at: lib/Sema/SemaCUDA.cpp:97
+template <typename A>
+static bool getAttr(const FunctionDecl *D, bool IgnoreImplicitAttr) {
+  if (Attr *Attribute = D->getAttr<A>()) {
----------------
hasAttr?


================
Comment at: lib/Sema/SemaCUDA.cpp:900
+  }
+}
----------------
I don't see where this function is declared?


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5628
   case AttributeList::AT_CUDAHost:
-    handleSimpleAttributeWithExclusions<CUDAHostAttr, CUDAGlobalAttr>(S, D,
-                                                                      Attr);
+    if (!D->hasAttr<CUDAHostAttr>())
+      handleSimpleAttributeWithExclusions<CUDAHostAttr, CUDAGlobalAttr>(S, D,
----------------
Is this a functional change in this patch?  We don't check for duplicates of most other attributes, so I'm not sure why it should matter with these.  If it does matter, we should have comments...


================
Comment at: lib/Sema/SemaTemplate.cpp:7046
       if (LangOpts.CUDA &&
-          IdentifyCUDATarget(Specialization) != IdentifyCUDATarget(FD)) {
+          IdentifyCUDATarget(Specialization, true) !=
+              IdentifyCUDATarget(FD, true)) {
----------------
http://jlebar.com/2011/12/16/Boolean_parameters_to_API_functions_considered_harmful..html  :)

At least please do

  IdentifyCUDATarget(Specialization, /* IgnoreImplicitTargetAttrs = */ true);


================
Comment at: lib/Sema/SemaTemplate.cpp:7168
+  if (LangOpts.CUDA)
+    mergeCUDATargetAttributes(FD, Specialization);
   return false;
----------------
I am unsure of whether or why we need this anymore, since I don't see this declared in any header, and also since I thought we weren't merging attrs anymore?


================
Comment at: lib/Sema/SemaTemplate.cpp:8119
     if (LangOpts.CUDA &&
-        IdentifyCUDATarget(Specialization) != IdentifyCUDATarget(Attr)) {
+        IdentifyCUDATarget(Specialization, true) != IdentifyCUDATarget(Attr)) {
       FailedCandidates.addCandidate().set(
----------------
Same here wrt bool param.


https://reviews.llvm.org/D25845