fix: incorrect code generation for parameter borrowing #1460
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Querying whether and what kind of view a type represents has nothing to do with the "type allowed" checks.
Both `mirgen` and `isPassByRef` didn't consider non-direct views when deciding whether the parameter uses pass-by-reference. Now they do.
zerbina
added
bug
saem
approved these changes
Sep 11, 2024
| result = classifyViewTypeAux(marker, t) | ||
|
|
||
| proc directViewType*(t: PType): ViewTypeKind = | ||
| # does classify 't' without looking recursively into 't'. |
There was a problem hiding this comment.
Suggested change
| # does classify 't' without looking recursively into 't'. | |
| ## classifies 't' without looking recursively into 't'. |
A tad clearer and shorter, also made it a doc comment as it's relevant to what to expect from the procedure.
There was a problem hiding this comment.
I agree, but I figured to not modify the moved-over code, so as to allow git tooling to better detect the moved code.
|
/merge |
|
Merge requested by: @saem Contents after the first section break of the PR description has been removed and preserved below: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fix parameters below the pass-by-reference size threshold not being
passed by reference when the procedure returns a non-direct view,
resulting in access violations at run-time when trying to access the
returned view.
Details
objecttypes withlentfields) when deciding whether pass-by-reference is used for the first
parameter of a procedure
classifyViewTypeprocedures fromtypeallowedtotypes,as they're wholly unrelated to the "type allowed" checks
Fixes #1457.