[PATCH] Fix PR20495: correct inference of the CUDA target for implicit members

Eli Bendersky eliben at google.com
Tue Sep 9 08:40:25 PDT 2014


Thanks for the review. PTAL

================
Comment at: lib/Sema/SemaCUDA.cpp:123-133
@@ +122,13 @@
+  llvm::SmallVector<const CXXBaseSpecifier *, 16> Bases;
+  for (const auto &B : ClassDecl->bases()) {
+    if (!ClassDecl->isAbstract() || !B.isVirtual()) {
+      Bases.push_back(&B);
+    }
+  }
+
+  if (!ClassDecl->isAbstract()) {
+    for (const auto &VB : ClassDecl->vbases()) {
+      Bases.push_back(&VB);
+    }
+  }
+
----------------
rsmith wrote:
> For a non-abstract class, you'll collect direct vbases twice here (once from the bases list and once from the vbases list). The easiest thing to do would be to change the condition in the first loop to just
> 
>   if (!B.isVirtual())
Done

================
Comment at: lib/Sema/SemaCUDA.cpp:164
@@ +163,3 @@
+          Diag(ClassDecl->getLocation(),
+               diag::err_implicit_member_target_infer_collision)
+              << (unsigned)CSM << InferredTarget.getValue()
----------------
rsmith wrote:
> This should be a note, rather than an error.
Done

================
Comment at: lib/Sema/SemaDeclCXX.cpp:5568-5574
@@ +5567,9 @@
+    // failed.
+    bool Const = false;
+    if ((CSM == CXXCopyConstructor &&
+         RD->implicitCopyConstructorHasConstParam()) ||
+        (CSM == CXXCopyAssignment &&
+         RD->implicitCopyAssignmentHasConstParam())) {
+      Const = true;
+    }
+    return inferCUDATargetForImplicitSpecialMember(RD, CSM, MD, Const,
----------------
rsmith wrote:
> This is `SMI.ConstArg`.
Done

================
Comment at: lib/Sema/SemaOverload.cpp:5638
@@ +5637,3 @@
+      // Skip the check for callers that are implicit members, because in this
+      // case we still don't know what the member's target is; the target is
+      // inferred for the member automatically, based on the bases and fields of
----------------
rsmith wrote:
> s/we still don't know/we may not yet know/
Done

http://reviews.llvm.org/D5199






More information about the cfe-commits mailing list