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

all: make type of mut function args and loop values consistent #8802

Closed
wants to merge 124 commits into from

Conversation

UweKrueger
Copy link
Member

@UweKrueger UweKrueger commented Feb 17, 2021

Function arguments, receivers and for loop variables that are declared mut are references behind the scenes. However it is not clear what their logical type is, i.e. if the behave like values or like references. The present state is like this:

fn f(mut y St) &St {
    println(typeof(y).name) // -> &St
    mut x := y // `y` behaves like a value, i.e. `x` is of type `St`
    mut z := [y] // again `y` behaves like a value, i.e. `z` is of type []St
    return y // here `y` behaves like a reference (`return &y` would not be allowed)
}

This is an inconsistent behaviour, which is made possible by a bunch of exceptions in checker and cgen.

This PR changes the state of mut arguments so that they are logically pure values. In particular typeof(y).name would become St and return &y would be required. This eventually reduces the number of exceptions required in checker and limits them to cgen. (At the present state there are some exception that had to be introduced in checker to make existing code continue to work, but these will become warnings and can be removed later.)

This fixes #11038.

@SleepyRoy
Copy link
Member

@yuyi98

@yuyi98
Copy link
Member

yuyi98 commented Jul 26, 2021

@UweKrueger I hope the conflict can be resolved as soon as possible and the merger can take place as soon as possible!

@JalonSolov
Copy link
Contributor

Only CI failure was in MacOS compiling V UI - that's been fixed now, hasn't it?

Taking care of the conflicts should allow this to be ready to merge.

@UweKrueger
Copy link
Member Author

Taking care of the conflicts should allow this to be ready to merge.

@JalonSolov Unfortunately it's not that trivial. A lot of code has been contributed in the last few weeks that causes semantic conflicts.
In particular there has been significant progress in the JS-backend (thanks to @playXE 👍) that was done with the old behaviour in mind.
For this reason I'm reconsidering my approach: Even though the checker does its checks with the logical types it should be somehow possible to put the low-level type into the AST. That would minimize the changes in the backends...

@UweKrueger UweKrueger marked this pull request as draft August 24, 2021 18:15
@danieldaeschle
Copy link
Member

Any progress? :/

@JalonSolov JalonSolov added the Needs Rebase The code of the PR must be rebased over current master before it can be approved. label Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Rebase The code of the PR must be rebased over current master before it can be approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arithmetic in mutating for loop works on pointers
6 participants