[clang] 176981a - [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

Takuya Shimizu via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 14 07:00:51 PDT 2023


Author: Takuya Shimizu
Date: 2023-07-14T22:59:41+09:00
New Revision: 176981ac58d3e4beb02b9d110524e72be509375d

URL: https://github.com/llvm/llvm-project/commit/176981ac58d3e4beb02b9d110524e72be509375d
DIFF: https://github.com/llvm/llvm-project/commit/176981ac58d3e4beb02b9d110524e72be509375d.diff

LOG: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

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.
This patch fixes it by changing the source range from function callsite to the problematic parameter in function declaration.

e.g.

```
void func(int aa, int bb);

void test() { func(1, "two"); }
```
BEFORE this patch:

```
source:4:15: error: no matching function for call to 'func'
    4 | void test() { func(1, "two"); }
      |               ^~~~
source:1:6: note: candidate function not viable: no known conversion from 'const char[4]' to 'int' for 2nd argument
    1 | void func(int aa, int bb);
      |      ^
    2 |
    3 |
    4 | void test() { func(1, "two"); }
      |                       ~~~~~
1 error generated.
```
AFTER this patch:

```
source:4:15: error: no matching function for call to 'func'
    4 | void test() { func(1, "two"); }
      |               ^~~~
source:1:6: note: candidate function not viable: no known conversion from 'const char[4]' to 'int' for 2nd argument
    1 | void func(int aa, int bb);
      |      ^            ~~~~~~
```

Reviewed By: cjdb
Differential Revision: https://reviews.llvm.org/D153359

Added: 
    clang/test/Misc/diag-overload-cand-ranges.cpp
    clang/test/Misc/diag-overload-cand-ranges.mm

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaOverload.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 08d49bf72fbaaf..53a83b9c94176d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -384,6 +384,37 @@ Improvements to Clang's diagnostics
   (`#57081: <https://github.com/llvm/llvm-project/issues/57081>`_)
 - Clang no longer emits inappropriate notes about the loss of ``__unaligned`` qualifier
   on overload resolution, when the actual reason for the failure is loss of other qualifiers.
+- Clang's notes about unconvertible types in overload resolution failure now covers
+  the source range of parameter declaration of the candidate function declaration.
+
+  *Example Code*:
+
+  .. code-block:: c++
+
+     void func(int aa, int bb);
+     void test() { func(1, "two"); }
+
+  *BEFORE*:
+
+  .. code-block:: text
+
+    source:2:15: error: no matching function for call to 'func'
+    void test() { func(1, "two");  }
+                  ^~~~
+    source:1:6: note: candidate function not viable: no known conversion from 'const char[4]' to 'int' for 2nd argument
+    void func(int aa, int bb);
+         ^
+
+  *AFTER*:
+
+  .. code-block:: text
+
+    source:2:15: error: no matching function for call to 'func'
+    void test() { func(1, "two");  }
+                  ^~~~
+    source:1:6: note: candidate function not viable: no known conversion from 'const char[4]' to 'int' for 2nd argument
+    void func(int aa, int bb);
+         ^            ~~~~~~
 
 Bug Fixes in This Version
 -------------------------

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d45257ec7a4c7b..45a9e5dc98c032 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10753,6 +10753,8 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
   Expr *FromExpr = Conv.Bad.FromExpr;
   QualType FromTy = Conv.Bad.getFromType();
   QualType ToTy = Conv.Bad.getToType();
+  SourceRange ToParamRange =
+      !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() : SourceRange();
 
   if (FromTy == S.Context.OverloadTy) {
     assert(FromExpr && "overload set argument came from implicit argument?");
@@ -10763,8 +10765,7 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
 
     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;
+        << ToParamRange << ToTy << Name << I + 1;
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
   }
@@ -10793,14 +10794,12 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
       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 << ToParamRange << FromQs.getAddressSpace()
+            << ToQs.getAddressSpace() << ToTy->isReferenceType() << I + 1;
       MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
       return;
     }
@@ -10808,9 +10807,8 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
     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
-          << FromQs.getObjCLifetime() << ToQs.getObjCLifetime()
-          << (unsigned)isObjectArgument << I + 1;
+          << ToParamRange << FromTy << FromQs.getObjCLifetime()
+          << ToQs.getObjCLifetime() << (unsigned)isObjectArgument << I + 1;
       MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
       return;
     }
@@ -10818,9 +10816,8 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
     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
-          << FromQs.getObjCGCAttr() << ToQs.getObjCGCAttr()
-          << (unsigned)isObjectArgument << I + 1;
+          << ToParamRange << FromTy << FromQs.getObjCGCAttr()
+          << ToQs.getObjCGCAttr() << (unsigned)isObjectArgument << I + 1;
       MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
       return;
     }
@@ -10831,13 +10828,11 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
     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;
+          << ToParamRange << FromTy << (CVR - 1) << I + 1;
     }
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
@@ -10849,7 +10844,7 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
         << (unsigned)isObjectArgument << I + 1
         << (Conv.Bad.Kind == BadConversionSequence::rvalue_ref_to_lvalue)
-        << (FromExpr ? FromExpr->getSourceRange() : SourceRange());
+        << ToParamRange;
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
   }
@@ -10859,8 +10854,7 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
   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
-        << (unsigned)isObjectArgument << I + 1
+        << ToParamRange << FromTy << ToTy << (unsigned)isObjectArgument << I + 1
         << (Conv.Bad.Kind == BadConversionSequence::too_few_initializers ? 1
             : Conv.Bad.Kind == BadConversionSequence::too_many_initializers
                 ? 2
@@ -10879,8 +10873,7 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
     // 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
+        << ToParamRange << FromTy << ToTy << (unsigned)isObjectArgument << I + 1
         << (unsigned)(Cand->Fix.Kind);
 
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
@@ -10921,24 +10914,24 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
   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())
-        << (BaseToDerivedConversion - 1) << FromTy << ToTy << I + 1;
+        << ToParamRange << (BaseToDerivedConversion - 1) << FromTy << ToTy
+        << I + 1;
     MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
     return;
   }
 
   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
+          << ToParamRange << FromTy << ToTy << (unsigned)isObjectArgument
+          << I + 1;
+      MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
+      return;
+    }
   }
 
   if (TakingCandidateAddress &&
@@ -10948,8 +10941,7 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
   // 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
+        << ToParamRange << FromTy << ToTy << (unsigned)isObjectArgument << I + 1
         << (unsigned)(Cand->Fix.Kind);
 
   // Check that location of Fn is not in system header.

diff  --git a/clang/test/Misc/diag-overload-cand-ranges.cpp b/clang/test/Misc/diag-overload-cand-ranges.cpp
new file mode 100644
index 00000000000000..080ca484d4b746
--- /dev/null
+++ b/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); }

diff  --git a/clang/test/Misc/diag-overload-cand-ranges.mm b/clang/test/Misc/diag-overload-cand-ranges.mm
new file mode 100644
index 00000000000000..6b9b849b0997d1
--- /dev/null
+++ b/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))
+ at interface Interface
+- (void)something;
+ at end
+
+ at implementation Interface
+- (void)something{
+    func(self);
+}
+ at end


        


More information about the cfe-commits mailing list