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?

include/clang/Basic/DiagnosticSemaKinds.td

     "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 |"

lib/Sema/SemaOverload.cpp

   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;
   }

test/Parser/cxx-reference.cpp

 #if __cplusplus <= 199711L
 // expected-warning@-2 {{rvalue references are a C++11 extension}}
 #endif
+
+namespace PointerVsReferenceSuggestion{
+  class A;
+  
+  void f0(A *a); // expected-note {{candidate function not viable: cannot convert argument of incomplete type 'PointerVsReferenceSuggestion::A' to 'PointerVsReferenceSuggestion::A *' for 1st argument; take the address of the argument with &}}
+  void f1(A &a) {
+    f0(a); // expected-error {{no matching function for call to 'f0'}}
+  }
+  
+  void f2(A &a); // expected-note {{candidate function not viable: cannot convert argument of incomplete type 'PointerVsReferenceSuggestion::A *' to 'PointerVsReferenceSuggestion::A &' for 1st argument; dereference the argument with *}}
+  void f3(A *a) {
+    f2(a); // expected-error {{no matching function for call to 'f2'}}
+  }
+  
+  class B {};
+  
+  void f4(B *b); // expected-note {{candidate function not viable: no known conversion from 'PointerVsReferenceSuggestion::B' to 'PointerVsReferenceSuggestion::B *' for 1st argument; take the address of the argument with &}}
+  void f5(B &b) {
+    f4(b); // expected-error {{no matching function for call to 'f4'}}
+  }
+  
+  void f6(B &b); // expected-note {{candidate function not viable: no known conversion from 'PointerVsReferenceSuggestion::B *' to 'PointerVsReferenceSuggestion::B &' for 1st argument; dereference the argument with *}}
+  void f7(B *b) {
+    f6(b); // expected-error {{no matching function for call to 'f6'}}
+  }
+}