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

Add resolvedOrVirtualFunction, fix issue #6542 #7332

Merged
merged 1 commit into from
Sep 18, 2017

Conversation

mppf
Copy link
Member

@mppf mppf commented Sep 15, 2017

This PR takes two steps to fix issue #6542:

  1. It introduces a function, CallExpr::resolvedOrVirtualFunction, factoring out a code pattern used already in callDestructors and errorHandling. The pattern is to find the resolved function, or, for a PRIM_VIRTUAL_METHOD_CALL, find one of the possible virtual methods that could be called at runtime. This function is appropriate to call for code that identifies calls to transform based upon the signature of the called function.
  2. It calls this function in two places in order to resolve issue Returning by-ref values from overridden methods and not capturing them at call site #6542.

What was going wrong with issue #6542? The compiler includes a transformation that happens during resolution, insertReturnTemps, which finds all calls to functions that return a value that don't use the result of that return. In that case it adds a temporary variable to capture the returned value.

Code in callDestructors / ReturnByRef relied on calls to transformable functions having a variable to capture the returned value.

In the case of virtual method calls, insertReturnTemps was not transforming the call, and so the code pattern in issue #6542 led to failed assertions in callDestructors.

The important part of this fix is to adjust insertReturnTemps to handle virtual calls (PRIM_VIRTUAL_METHOD_CALL).

Passed full local testing.
Reviewed by @noakesmichael - thanks!

@mppf
Copy link
Member Author

mppf commented Sep 15, 2017

@noakesmichael - would you mind looking at this bug fix? Thanks.

@mppf mppf merged commit d29eae0 into chapel-lang:master Sep 18, 2017
@mppf mppf deleted the fix-6542 branch September 18, 2017 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant