Fix for: Bug 5941 - improve diagnostic for * vs & confusion

[ryee88]

Summary

This is bug 5941: https://llvm.org/bugs/show_bug.cgi?id=5941

I copied the extra suggestions from the complete type case to the incomplete type case.

[ryee88]

Ping-- can someone look at this review?

[dblaikie]

Needs test coverage

[ryee88]

Added unit test as per review comments.

[ryee88]

Sorry-- re-uploading the diff. Didn't notice I dropped the code changes.

[dblaikie]

Since this is just a wording change, presumably the diagnostic is already
tested in an existing file - perhaps you could exetendi that test to cover
this functionality (we try not to add new test files too much - better to
test functionality once/in one place as much as possible (both for ease of
reading/updating/understanding the test suite, and for speed of execution
(process startup time makes up a significant portion of the test execution
time, so reducing the total number of test files/commands is useful there)))

[ryee88]

Keeping the number of test files to a minimum makes sense.

Couldn't find an existing test for this diagnostic. I did find a cxx-reference.cpp that tests reference diagnostics so this seems like a reasonable location for this new test.

[ryee88]

any comments?

[ryee88]

Moved test to a file that checks for similar diagnostics.

[dblaikie]

Are there not any existing cases that test this diagnostic?

Could you just update some existing cases to cover this? (it looks like, in the same file, the test case added for PR 6117 could be expanded to test this as well, perhaps?) It looks like that's the original test for this diagnostic, so it'd be a good/right place to test the extra text and add any extra variants that have become relevant.

[ryee88]

Yeah you’re right about this one being the original variant. I missed it when I was reading through the tests in this file.

I’ve reworked it to reflect the new variants.

[dblaikie]

Looks great - thanks!

[ryee88]

Thanks! I don’t have commit permissions; can you submit for me?

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def note_ovl_candidate_bad_conv_incomplete : Note<"candidate "
     "function |function |constructor |"
     "constructor (the implicit default constructor)|"
     "constructor (the implicit copy constructor)|"
     "constructor (the implicit move constructor)|"
     "function (the implicit copy assignment operator)|"
     "function (the implicit move assignment operator)|"
     "constructor (inherited)}0%1 "
     "not viable: cannot convert argument of incomplete type "
-    "%diff{$ to $|to parameter type}2,3">;
+    "%diff{$ to $|to parameter type}2,3 for "
+    "%select{%ordinal5 argument|object argument}4"
+    "%select{|; dereference the argument with *|"
+    "; take the address of the argument with &|"
+    "; remove *|"
+    "; remove &}6">;
 def note_ovl_candidate_bad_list_argument : Note<"candidate "
     "%select{function|function|constructor|"
     "function |function |constructor |"
     "constructor (the implicit default constructor)|"
     "constructor (the implicit copy constructor)|"
     "constructor (the implicit move constructor)|"
     "function (the implicit copy assignment operator)|"
     "function (the implicit move assignment operator)|"

cfe/trunk/lib/Sema/SemaOverload.cpp

static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
 
   // Diagnose references or pointers to incomplete types differently,
   // since it's far from impossible that the incompleteness triggered
   // the failure.
   QualType TempFromTy = FromTy.getNonReferenceType();
   if (const PointerType *PTy = TempFromTy->getAs<PointerType>())
     TempFromTy = PTy->getPointeeType();
   if (TempFromTy->isIncompleteType()) {
+    // Emit the generic diagnostic and, optionally, add the hints to it.
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_conv_incomplete)
       << (unsigned) FnKind << FnDesc
       << (FromExpr ? FromExpr->getSourceRange() : SourceRange())
-      << FromTy << ToTy << (unsigned) isObjectArgument << I+1;
+      << FromTy << ToTy << (unsigned) isObjectArgument << I+1
+      << (unsigned) (Cand->Fix.Kind);
+      
     MaybeEmitInheritedConstructorNote(S, Fn);
     return;
   }
 
   // Diagnose base -> derived pointer conversions.
   unsigned BaseToDerivedConversion = 0;
   if (const PointerType *FromPtrTy = FromTy->getAs<PointerType>()) {
     if (const PointerType *ToPtrTy = ToTy->getAs<PointerType>()) {

cfe/trunk/test/SemaCXX/overload-call.cpp

class qrgb666 {
     inline operator unsigned int () const;
 
     inline bool operator==(const qrgb666 &v) const;
     inline bool operator!=(const qrgb666 &v) const { return !(*this == v); }
   };
 }
 
 // PR 6117
-namespace test3 {
-  struct Base {};
+namespace IncompleteConversion {
+  struct Complete {};
   struct Incomplete;
 
-  void foo(Base *); // expected-note 2 {{cannot convert argument of incomplete type}}
-  void foo(Base &); // expected-note 2 {{cannot convert argument of incomplete type}}
+  void completeFunction(Complete *); // expected-note 2 {{cannot convert argument of incomplete type}}
+  void completeFunction(Complete &); // expected-note 2 {{cannot convert argument of incomplete type}}
   
-  void test(Incomplete *P) {
-    foo(P); // expected-error {{no matching function for call to 'foo'}}
-    foo(*P); // expected-error {{no matching function for call to 'foo'}}
+  void testTypeConversion(Incomplete *P) {
+    completeFunction(P); // expected-error {{no matching function for call to 'completeFunction'}}
+    completeFunction(*P); // expected-error {{no matching function for call to 'completeFunction'}}
+  }
+  
+  void incompletePointerFunction(Incomplete *); // expected-note {{candidate function not viable: cannot convert argument of incomplete type 'IncompleteConversion::Incomplete' to 'IncompleteConversion::Incomplete *' for 1st argument; take the address of the argument with &}}
+  void incompleteReferenceFunction(Incomplete &); // expected-note {{candidate function not viable: cannot convert argument of incomplete type 'IncompleteConversion::Incomplete *' to 'IncompleteConversion::Incomplete &' for 1st argument; dereference the argument with *}}
+  
+  void testPointerReferenceConversion(Incomplete &reference, Incomplete *pointer) {
+    incompletePointerFunction(reference); // expected-error {{no matching function for call to 'incompletePointerFunction'}}
+    incompleteReferenceFunction(pointer); // expected-error {{no matching function for call to 'incompleteReferenceFunction'}}
   }
 }
 
 namespace DerivedToBaseVsVoid {
   struct A { };
   struct B : A { };
   
   float &f(void *);