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

borrows of parameters below byref threshold ends up leasing the copied parameter #1457

Closed
alaviss opened this issue Sep 10, 2024 · 0 comments · Fixed by #1460
Closed

borrows of parameters below byref threshold ends up leasing the copied parameter #1457

alaviss opened this issue Sep 10, 2024 · 0 comments · Fixed by #1460
Assignees
Labels
bug Something isn't working compiler/backend Related to backend system of the compiler

Comments

@alaviss
Copy link
Contributor

alaviss commented Sep 10, 2024

Example

{.experimental: "views".}
type
  Mint = object
    data: int

  Option = object
    value: lent Mint

proc `=copy`(dst: var Mint, src: Mint) {.error.}

proc some(x: Mint): Option =
  Option(value: x)

proc main() =
  var x = Mint(data: 10)
  let optlent = some(x)

main()

Actual Output

C Code for some(x):

N_LIB_PRIVATE N_NIMCALL(_L6Option_2_M4test, some__test_8)(_L4Mint_1_M4test x) {
	_L6Option_2_M4test result;
	_L4Mint_1_M4test* _2;
	nimfr_("some", "/tmp/test.nim");
	nimln_(12, "/tmp/test.nim");
	_2 = (&x);
	nimZeroMem((void*)(&result), sizeof(_L6Option_2_M4test));
	result.value = _2;
	popFrame();
	return result;
}

Expected Output

The x parameter should be a pointer, not a stack copy.

Additional information

  • If Mint is tagged with {.byref.}, the generated code is correct.
@alaviss alaviss added bug Something isn't working compiler/backend Related to backend system of the compiler labels Sep 10, 2024
@zerbina zerbina self-assigned this Sep 10, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 11, 2024
## 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

* consider immutable non-direct views (e.g., `object` types with `lent`
  fields) when deciding whether pass-by-reference is used for the first
  parameter of a procedure
* move the `classifyViewType` procedures from `typeallowed` to `types`,
  as they're wholly unrelated to the "type allowed" checks

Fixes #1457.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/backend Related to backend system of the compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants