hazohelet created this revision.
hazohelet added reviewers: aaron.ballman, tbaeder, cjdb, shafik.
Herald added a project: All.
hazohelet requested review of this revision.
Herald added a project: clang.

Now that clang supports printing of multiple lines of code snippet in 
diagnostics, source ranges in diagnostics that are located in different lines 
from the diagnosed source location get to be printed if the gap happened to be 
less than the maximum number of lines clang is allowed to print in.
Many of the bad-conversion notes in overload resolution failures have their 
source location in the function declaration and source range in the argument of 
function call. This can cause an unnecessarily many lines of snippet printing.
e.g.

  void func(int num);
  
  
  void test() { func("hello"); }

Live demo: https://godbolt.org/z/fdj6WWsef

This patch fixes it by changing the source range from function callsite to the 
problematic parameter in function declaration.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153359

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Misc/diag-overload-cand-ranges.cpp
  clang/test/Misc/diag-overload-cand-ranges.mm

Index: clang/test/Misc/diag-overload-cand-ranges.mm
===================================================================
--- /dev/null
+++ clang/test/Misc/diag-overload-cand-ranges.mm
@@ -0,0 +1,26 @@
+// RUN: not %clang_cc1 -fobjc-runtime-has-weak -fobjc-arc -fsyntax-only -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s --strict-whitespace -check-prefixes=CHECK,ARC
+// RUN: not %clang_cc1 -fobjc-runtime-has-weak -fobjc-gc -fsyntax-only -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s --strict-whitespace -check-prefixes=CHECK,GC
+
+// CHECK:      error: no matching function
+// CHECK:      :{[[@LINE+1]]:15-[[@LINE+1]]:28}: note: {{.*}}: 1st argument
+void powerful(__strong id &);
+void lifetime_gcattr_mismatch() {
+  static __weak id weak_id;
+  powerful(weak_id);
+}
+
+// CHECK:      error: no matching function
+// ARC:        :{[[@LINE+2]]:11-[[@LINE+2]]:21}: note: {{.*}}: cannot implicitly convert
+// GC:         :{[[@LINE+1]]:11-[[@LINE+1]]:21}: note: {{.*}}: no known conversion
+void func(char *uiui);
+
+__attribute__((objc_root_class))
+@interface Interface
+- (void)something;
+@end
+
+@implementation Interface
+- (void)something{
+    func(self);
+}
+@end
Index: clang/test/Misc/diag-overload-cand-ranges.cpp
===================================================================
--- /dev/null
+++ clang/test/Misc/diag-overload-cand-ranges.cpp
@@ -0,0 +1,72 @@
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s --strict-whitespace
+// CHECK:      error: no matching function
+template <typename T> struct mcdata {
+  typedef int result_type;
+};
+template <class T> typename mcdata<T>::result_type wrap_mean(mcdata<T> const &);
+// CHECK:      :{[[@LINE+1]]:19-[[@LINE+1]]:53}: note: {{.*}}: no overload of 'wrap_mean'
+void add_property(double (*)(mcdata<double> const &));
+void f() { add_property(&wrap_mean); }
+
+// CHECK:      error: no matching function
+// CHECK:      :{[[@LINE+1]]:10-[[@LINE+1]]:51}: note: {{.*}}: cannot pass pointer to generic address space
+void baz(__attribute__((opencl_private)) int *Data) {}
+void fizz() {
+  int *Nop;
+  baz(Nop);
+  // CHECK:    error: no matching function
+  // CHECK:    :[[@LINE+1]]:53: note: {{.*}}: 'this' object is in address space '__private'
+  __attribute__((opencl_private)) static auto err = [&]() {};
+  err();
+}
+
+// CHECK:      error: no matching function
+struct Bar {
+// CHECK:      :{[[@LINE+1]]:26-[[@LINE+1]]:32}: note: {{.*}} would lose const qualifier
+static void foo(int num, int *X) {}
+// CHECK:      :{[[@LINE+1]]:17-[[@LINE+1]]:25}: note: {{.*}} no known conversion
+static void foo(int *err, int *x) {}
+};
+void bar(const int *Y) {
+  Bar::foo(5, Y);
+}
+
+struct InComp;
+
+struct A {};
+struct B : public A{};
+// CHECK:      error: no matching function
+// CHECK:      :{[[@LINE+5]]:36-[[@LINE+5]]:50}: note: {{.*}}: cannot convert initializer
+// CHECK:      error: no matching function
+// CHECK:      :{[[@LINE+3]]:36-[[@LINE+3]]:50}: note: {{.*}}: cannot convert argument
+// CHECK:      error: no matching function
+// CHECK:      :{[[@LINE+1]]:11-[[@LINE+1]]:18}: note: {{.*}}: no known conversion
+void hoge(char aa, const char *bb, const A& third);
+
+// CHECK:      error: no matching function
+// CHECK:      :{[[@LINE+1]]:14-[[@LINE+1]]:16}: note: {{.*}}: cannot convert from base class
+void derived(B*);
+
+void func(const A &arg) {
+  hoge(1, "pass", {{{arg}}});
+  InComp *a;
+  hoge(1, "pass", a);
+  hoge("first", 5, 6);
+  A *b;
+  derived(b);
+}
+
+struct Q {
+  // CHECK:    error: invalid operands
+  // CHECK:    :[[@LINE+1]]:6: note: {{.*}}: 'this' argument has type 'const Q'
+  Q &operator+(void*);
+};
+
+void fuga(const Q q) { q + 3; }
+
+template <short T> class Type1 {};
+// CHECK:      error: no matching function
+// CHECK:      :{[[@LINE+1]]:43-[[@LINE+1]]:54}: note: {{.*}}: expects an lvalue
+template <short T> void Function1(int zz, Type1<T> &x, int ww) {}
+
+void Function() { Function1(33, Type1<-42>(), 66); }
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10749,6 +10749,7 @@
   Expr *FromExpr = Conv.Bad.FromExpr;
   QualType FromTy = Conv.Bad.getFromType();
   QualType ToTy = Conv.Bad.getToType();
+  ParmVarDecl *ToPVD = !isObjectArgument ? Fn->getParamDecl(I) : nullptr;
 
   if (FromTy == S.Context.OverloadTy) {
     assert(FromExpr && "overload set argument came from implicit argument?");
@@ -10759,8 +10760,8 @@
 
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_overload)
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-        << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << ToTy
-        << Name << I + 1;
+        << (ToPVD ? ToPVD->getSourceRange() : SourceRange()) << ToTy << Name
+        << I + 1;
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
   }
@@ -10789,14 +10790,12 @@
       if (isObjectArgument)
         S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_addrspace_this)
             << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second
-            << FnDesc << (FromExpr ? FromExpr->getSourceRange() : SourceRange())
-            << FromQs.getAddressSpace() << ToQs.getAddressSpace();
+            << FnDesc << FromQs.getAddressSpace() << ToQs.getAddressSpace();
       else
         S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_addrspace)
             << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second
-            << FnDesc << (FromExpr ? FromExpr->getSourceRange() : SourceRange())
-            << FromQs.getAddressSpace() << ToQs.getAddressSpace()
-            << ToTy->isReferenceType() << I + 1;
+            << FnDesc << ToPVD->getSourceRange() << FromQs.getAddressSpace()
+            << ToQs.getAddressSpace() << ToTy->isReferenceType() << I + 1;
       MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
       return;
     }
@@ -10804,7 +10803,7 @@
     if (FromQs.getObjCLifetime() != ToQs.getObjCLifetime()) {
       S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_ownership)
           << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-          << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
+          << (ToPVD ? ToPVD->getSourceRange() : SourceRange()) << FromTy
           << FromQs.getObjCLifetime() << ToQs.getObjCLifetime()
           << (unsigned)isObjectArgument << I + 1;
       MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
@@ -10814,13 +10813,14 @@
     if (FromQs.getObjCGCAttr() != ToQs.getObjCGCAttr()) {
       S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_gc)
           << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-          << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
+          << (ToPVD ? ToPVD->getSourceRange() : SourceRange()) << FromTy
           << FromQs.getObjCGCAttr() << ToQs.getObjCGCAttr()
           << (unsigned)isObjectArgument << I + 1;
       MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
       return;
     }
 
+    // FIXME: No test case for this. Can we remove this block?
     if (FromQs.hasUnaligned() != ToQs.hasUnaligned()) {
       S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_unaligned)
           << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
@@ -10836,13 +10836,11 @@
     if (isObjectArgument) {
       S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_cvr_this)
           << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-          << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
-          << (CVR - 1);
+          << FromTy << (CVR - 1);
     } else {
       S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_cvr)
           << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-          << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
-          << (CVR - 1) << I + 1;
+          << ToPVD->getSourceRange() << FromTy << (CVR - 1) << I + 1;
     }
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
@@ -10854,7 +10852,7 @@
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
         << (unsigned)isObjectArgument << I + 1
         << (Conv.Bad.Kind == BadConversionSequence::rvalue_ref_to_lvalue)
-        << (FromExpr ? FromExpr->getSourceRange() : SourceRange());
+        << (ToPVD ? ToPVD->getSourceRange() : SourceRange());
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
   }
@@ -10864,7 +10862,7 @@
   if (FromExpr && isa<InitListExpr>(FromExpr)) {
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_list_argument)
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-        << FromExpr->getSourceRange() << FromTy << ToTy
+        << (ToPVD ? ToPVD->getSourceRange() : SourceRange()) << FromTy << ToTy
         << (unsigned)isObjectArgument << I + 1
         << (Conv.Bad.Kind == BadConversionSequence::too_few_initializers ? 1
             : Conv.Bad.Kind == BadConversionSequence::too_many_initializers
@@ -10884,9 +10882,8 @@
     // Emit the generic diagnostic and, optionally, add the hints to it.
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_conv_incomplete)
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-        << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
-        << ToTy << (unsigned)isObjectArgument << I + 1
-        << (unsigned)(Cand->Fix.Kind);
+        << (ToPVD ? ToPVD->getSourceRange() : SourceRange()) << FromTy << ToTy
+        << (unsigned)isObjectArgument << I + 1 << (unsigned)(Cand->Fix.Kind);
 
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
@@ -10926,7 +10923,7 @@
   if (BaseToDerivedConversion) {
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_base_to_derived_conv)
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-        << (FromExpr ? FromExpr->getSourceRange() : SourceRange())
+        << (ToPVD ? ToPVD->getSourceRange() : SourceRange())
         << (BaseToDerivedConversion - 1) << FromTy << ToTy << I + 1;
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
@@ -10934,16 +10931,16 @@
 
   if (isa<ObjCObjectPointerType>(CFromTy) &&
       isa<PointerType>(CToTy)) {
-      Qualifiers FromQs = CFromTy.getQualifiers();
-      Qualifiers ToQs = CToTy.getQualifiers();
-      if (FromQs.getObjCLifetime() != ToQs.getObjCLifetime()) {
-        S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_arc_conv)
-            << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second
-            << FnDesc << (FromExpr ? FromExpr->getSourceRange() : SourceRange())
-            << FromTy << ToTy << (unsigned)isObjectArgument << I + 1;
-        MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
-        return;
-      }
+    Qualifiers FromQs = CFromTy.getQualifiers();
+    Qualifiers ToQs = CToTy.getQualifiers();
+    if (FromQs.getObjCLifetime() != ToQs.getObjCLifetime()) {
+      S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_arc_conv)
+          << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
+          << (ToPVD ? ToPVD->getSourceRange() : SourceRange()) << FromTy << ToTy
+          << (unsigned)isObjectArgument << I + 1;
+      MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
+      return;
+    }
   }
 
   if (TakingCandidateAddress &&
@@ -10953,9 +10950,8 @@
   // Emit the generic diagnostic and, optionally, add the hints to it.
   PartialDiagnostic FDiag = S.PDiag(diag::note_ovl_candidate_bad_conv);
   FDiag << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
-        << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
-        << ToTy << (unsigned)isObjectArgument << I + 1
-        << (unsigned)(Cand->Fix.Kind);
+        << (ToPVD ? ToPVD->getSourceRange() : SourceRange()) << FromTy << ToTy
+        << (unsigned)isObjectArgument << I + 1 << (unsigned)(Cand->Fix.Kind);
 
   // Check that location of Fn is not in system header.
   if (!S.SourceMgr.isInSystemHeader(Fn->getLocation())) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to