[clang][Diagnostics] Provide parameter source range to arity-mismatch notes

[hazohelet]

Summary

Consider the following piece of code:

void func( int aa,
           int bb,
           int cc) {}

void arity_mismatch() {
  func(2, 4);
}

BEFORE:

source.cpp:6:3: error: no matching function for call to 'func'
    6 |   func(2, 4);
      |   ^~~~
source.cpp:1:6: note: candidate function not viable: requires 3 arguments, but 2 were provided
    1 | void func( int aa,
      |      ^

AFTER:

source.cpp:6:3: error: no matching function for call to 'func'
    6 |   func(2, 4);
      |   ^~~~
source.cpp:1:6: note: candidate function not viable: requires 3 arguments, but 2 were provided
    1 | void func( int aa,
      |      ^     ~~~~~~~
    2 |            int bb,
      |            ~~~~~~~
    3 |            int cc) {}
      |            ~~~~~~

[tbaeder]

LGTM but let's wait for a second opinion.

[cjdb]

I think this will be a good addition because it underscores the salient part of why the function isn't viable.

Would it be possible for patches like this to have both examples of before and after this patch in their commit message please? That'll help expose why patches like this are desirable. Happy to LGTM afterwards.

clang/test/Misc/diag-func-call-ranges.cpp

14

Why are you casting to void?

[aaron.ballman]

LGTM modulo Chris' comment, but can you please add a release note for the fix?

[hazohelet]

Address comments from @cjdb and @aaron.ballman

• Remove unnecessary cast to void in test
• Add release note

[cjdb]

Thanks! This LGTM now. Do you need assistance with merging?

[hazohelet]

In D153267#4445417, @cjdb wrote:

Thanks! This LGTM now. Do you need assistance with merging?

Thanks for the review! I have commit access now, so I'll be landing this myself.

clang/docs/ReleaseNotes.rst

- When diagnosing a constant expression where an enum without a fixed underlying
   type is set to a value outside the range of the enum's values, clang will now
   print the name of the enum in question.
 - Clang no longer diagnoses a read of an empty structure as use of an
   uninitialized variable.
   (`#26842: <https://github.com/llvm/llvm-project/issues/26842>`_)
 - The Fix-It emitted for unused labels used to expand to the next line, which caused
   visual oddities now that Clang shows more than one line of code snippet. This has
   been fixed and the Fix-It now only spans to the end of the ``:``.
+- Clang now underlines the parameter list of function declaration when emitting
+  a note about the mismatch in the number of arguments.
 
 Bug Fixes in This Version
 -------------------------
 
 - Fix segfault while running clang-rename on a non existing file.
   (`#36471 <https://github.com/llvm/llvm-project/issues/36471>`_)
 - Fix crash when diagnosing incorrect usage of ``_Nullable`` involving alias
   templates.

clang/lib/Sema/SemaExpr.cpp

if (Args.size() < MinArgs) {
         Diag(RParenLoc, MinArgs == NumParams && !Proto->isVariadic()
                             ? diag::err_typecheck_call_too_few_args
                             : diag::err_typecheck_call_too_few_args_at_least)
             << FnKind << MinArgs << static_cast<unsigned>(Args.size())
             << Fn->getSourceRange();
 
       // Emit the location of the prototype.
       if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
-        Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
+        Diag(FDecl->getLocation(), diag::note_callee_decl)
+            << FDecl << FDecl->getParametersSourceRange();
 
       return true;
     }
     // We reserve space for the default arguments when we create
     // the call expression, before calling ConvertArgumentsForCall.
     assert((Call->getNumArgs() == NumParams) &&
            "We should have reserved space for the default arguments before!");
   }
if (!Proto->isVariadic()) {
                  : diag::err_typecheck_call_too_many_args_at_most)
             << FnKind << NumParams << static_cast<unsigned>(Args.size())
             << Fn->getSourceRange()
             << SourceRange(Args[NumParams]->getBeginLoc(),
                            Args.back()->getEndLoc());
 
       // Emit the location of the prototype.
       if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
-        Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
+        Diag(FDecl->getLocation(), diag::note_callee_decl)
+            << FDecl << FDecl->getParametersSourceRange();
 
       // This deletes the extra arguments.
       Call->shrinkNumArgs(NumParams);
       return true;
     }
   }
   SmallVector<Expr *, 8> AllArgs;
   VariadicCallType CallType = getVariadicCallType(FDecl, Proto, Fn);

clang/lib/Sema/SemaOverload.cpp

static void DiagnoseArityMismatch(Sema &S, NamedDecl *Found, Decl *D,
 
   std::string Description;
   std::pair<OverloadCandidateKind, OverloadCandidateSelect> FnKindPair =
       ClassifyOverloadCandidate(S, Found, Fn, CRK_None, Description);
 
   if (modeCount == 1 && Fn->getParamDecl(0)->getDeclName())
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_arity_one)
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second
-        << Description << mode << Fn->getParamDecl(0) << NumFormalArgs;
+        << Description << mode << Fn->getParamDecl(0) << NumFormalArgs
+        << Fn->getParametersSourceRange();
   else
     S.Diag(Fn->getLocation(), diag::note_ovl_candidate_arity)
         << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second
-        << Description << mode << modeCount << NumFormalArgs;
+        << Description << mode << modeCount << NumFormalArgs
+        << Fn->getParametersSourceRange();
 
   MaybeEmitInheritedConstructorNote(S, Found);
 }
 
 /// Arity mismatch diagnosis specific to a function overload candidate.
 static void DiagnoseArityMismatch(Sema &S, OverloadCandidate *Cand,
                                   unsigned NumFormalArgs) {
   if (!CheckArityMismatch(S, Cand, NumFormalArgs))

clang/test/Misc/diag-func-call-ranges.c

+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s --strict-whitespace
+
+// CHECK:      :{9:3-9:7}: error: too few arguments
+// CHECK:      :{7:12-7:26}: note: 'func' declared here
+// CHECK:      :{10:3-10:7}{10:13-10:17}: error: too many arguments
+// CHECK:      :{7:12-7:26}: note: 'func' declared here
+void func( int aa, int bb) {}
+void arity_mismatch() {
+  func(3);
+  func(3, 4,5, 6);
+}

clang/test/Misc/diag-func-call-ranges.cpp

+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s --strict-whitespace
+
+// CHECK: error: no matching function for call to 'func'
+
+// CHECK:      :{[[@LINE+1]]:12-[[@LINE+1]]:18}: note: {{.*}} requires single argument
+void func( int aa ) {}
+// CHECK:      :{[[@LINE+1]]:12-[[@LINE+3]]:18}: note: {{.*}} requires 3 arguments
+void func( int aa,
+           int bb,
+           int cc) {}
+
+void arity_mismatch() {
+  func(2, 4);
+}