This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by hazohelet on Jun 19 2023, 5:19 AM.

Details

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) {}
      |            ~~~~~~

Diff Detail

Event Timeline

hazohelet created this revision.Jun 19 2023, 5:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2023, 5:19 AM
hazohelet requested review of this revision.Jun 19 2023, 5:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2023, 5:19 AM

LGTM but let's wait for a second opinion.

cjdb added a comment.Jun 21 2023, 11:30 AM

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
13

Why are you casting to void?

aaron.ballman accepted this revision.Jun 22 2023, 4:51 AM

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

This revision is now accepted and ready to land.Jun 22 2023, 4:51 AM
hazohelet updated this revision to Diff 533971.Jun 23 2023, 8:15 AM
hazohelet marked an inline comment as done.
hazohelet edited the summary of this revision. (Show Details)

Address comments from @cjdb and @aaron.ballman

  • Remove unnecessary cast to void in test
  • Add release note
cjdb accepted this revision.Jun 23 2023, 2:56 PM

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

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.