Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--explain-call to report multiple best candidates #25070

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vasslitvinov
Copy link
Member

This PR makes --explain-call option show all "best" candidates being considered in the case of return intent overloads. This may be useful when the user is expecting one overload to be invoked and does not realize that another overload is also being considered because of return intent overload.

Testing: standard and gasnet paratests.

Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Copy link
Member

@mppf mppf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me. One request: could have adjust it to print "best candidate is" (as it did before) if return intent overloading is not relevant (i.e., when only one of bestRef, bestValue, bestConstRef is not nullptr)?

@vasslitvinov
Copy link
Member Author

@mppf -

when only one of bestRef, bestValue, bestConstRef is not nullptr

My understanding is that in this case the function where my change is located is not invoked, instead the 1-candidate function is:

  if (numMatches == 0) {
    ....
  } else if (numMatches == 1) {
    ResolutionCandidate* best = NULL;

    if        (bestRef  != NULL) {
      best = bestRef;

    } else if (bestVal  != NULL) {
      best = bestVal;

    } else if (bestCref != NULL) {
      best = bestCref;
    }

    retval = resolveNormalCall(info, checkState, best);

  } else {
    // >>> my change is here <<<
    retval = resolveNormalCall(info, checkState, bestRef, bestCref, bestVal);
  }

What do you think?

@mppf
Copy link
Member

mppf commented May 20, 2024

That's fine; please just verify that the behavior is the way we expect when return intent overloading is used (it prints out the best candidate, not calling it ref or anything). We might have a test that already checks this (I did not check).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants