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

[Eli Bendersky]

Thanks for the review Richard. I believe I've addressed your comments. PTAL

================
Comment at: lib/Sema/SemaCUDA.cpp:110-111
@@ +109,4 @@
+                                                    bool ConstRHS) {
+  CUDAFunctionTarget InferredTarget;
+  bool HasInferredTarget = false;
+
----------------
rsmith wrote:
> `Optional<CUDAFunctionTarget>` InferredTarget;
Done.

================
Comment at: lib/Sema/SemaCUDA.cpp:119
@@ +118,3 @@
+  // Infer the target of this member base on the ones it should call.
+  for (const auto &B : ClassDecl->bases()) {
+    const RecordType *BaseType = B.getType()->getAs<RecordType>();
----------------
rsmith wrote:
> For a non-abstract class, you should visit virtual bases as well as direct bases. (For an abstract class, you should skip direct and indirect virtual bases.)
Done.

================
Comment at: lib/Sema/SemaCUDA.cpp:128-132
@@ +127,7 @@
+        LookupSpecialMember(BaseClassDecl, CSM,
+                            /* ConstArg */ ConstRHS,
+                            /* VolatileArg */ false,
+                            /* RValueThis */ false,
+                            /* ConstThis */ false,
+                            /* VolatileThis */ false);
+
----------------
rsmith wrote:
> rsmith wrote:
> > These are not necessarily correct; the user might have explicitly defaulted a const/volatile/whatever special member function.
> Hmm, I see you're not actually doing this for any defaulted function, just for an implicit one. In that case, this looks fine, but please rename the function to talk about implicit special members not defaulted ones.
Renaming done.

================
Comment at: lib/Sema/SemaCUDA.cpp:146-148
@@ +145,5 @@
+      if (ResolutionError) {
+        Diag(ClassDecl->getLocation(),
+             diag::err_implicit_member_target_infer_collision)
+            << (unsigned)CSM << InferredTarget << BaseMethodTarget;
+        return;
----------------
rsmith wrote:
> eliben wrote:
> > rsmith wrote:
> > > Declaring implicit special members is done lazily, so emitting diagnostics from here will result in erratic behavior. It would be better to defer the diagnostic until the special member is defined in C++98, and to mark the member as deleted in this case in C++11.
> > If I just mark the member as deleted in C++11, is there away to still emit the descriptive diagnostic? Just deleting the member doesn't provide the user with any hint of what the underlying problem is.
> Yes, see `Sema::NoteDeletedFunction`.
Thanks. I ended up implanting this into ShouldDeleteSpecialMember, with diagnostics; this is called from NoteDeletedFunction, and it also takes care of actually deleting the member in C++11 mode. Let me know if this makes sense.

================
Comment at: lib/Sema/SemaOverload.cpp:5637
@@ -5636,3 +5636,3 @@
     if (const FunctionDecl *Caller = dyn_cast<FunctionDecl>(CurContext))
-      if (CheckCUDATarget(Caller, Function)) {
+      if (!Caller->isImplicit() && CheckCUDATarget(Caller, Function)) {
         Candidate.Viable = false;
----------------
rsmith wrote:
> eliben wrote:
> > rsmith wrote:
> > > Hmm, why do you need this change?
> > Consider what happens when the target inference method runs. It looks up the ctors of base classes, which gets to this point. Now, at this point we don't yet know what the caller's (the implicit method being defined) target is, so we can not reason whether there's a target mismatch. 
> > 
> > AFAIU this added test solves the problem, since we really cannot reject candidates based on target-ness when declaring an implicit member. The implicit member's target-ness will be determined by the inference method, and the collision test (the one with the Diag you commented on) should take care of mismatches.
> > 
> > Does this make sense?
> Thanks, that makes sense. This at least deserves an explanatory comment.
Comment added, let me know if you'd want to see more details in it.

http://reviews.llvm.org/D5199