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

cgen: fix codegen for generated tmp var to fit number of indirections from fn argument #22752

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

felipensp
Copy link
Member

@felipensp felipensp commented Nov 3, 2024

Fix #22749

fn takes_ref(x &&&&int) {
	unsafe { ****x = 5 }
}

fn main() {
	y := 4
	takes_ref(y)
	println(y)
	assert y == 5
}
VV_LOCAL_SYMBOL void main__main(void) {
        int y = 4;
        int *_t1 = &y;
        int **_t1_1 = &_t1;
        int ***_t1_2 = &_t1_1;
        main__takes_ref((voidptr)&/*qq*/_t1_2);
        println(int_str(y));
}

Huly®: V_0.6-21198

@StunxFS
Copy link
Contributor

StunxFS commented Nov 4, 2024

I think code like that should not be allowed, at most it should be allowed if the argument is a simple reference type (&int).

In case the argument is of reference to reference type (&&int), the dev himself must pass the reference explicitly.

@medvednikov
Copy link
Member

It's a hard philosophical question. Many people in the community think there should be no autoref/deref, I think that a modern language should handle it.

@medvednikov
Copy link
Member

In 2025+ I don't want to remember, do I have to pass Config or &Config etc.

@medvednikov
Copy link
Member

But indeed maybe it should only be done for &Foo, but not for &&&&&Foo, which is too low level and rare.

@zeozeozeo
Copy link
Contributor

But indeed maybe it should only be done for &Foo, but not for &&&&&Foo, which is too low level and rare.

what about just &&Foo? having references to references is pretty common

@StunxFS
Copy link
Contributor

StunxFS commented Nov 4, 2024

@medvednikov

It's a hard philosophical question. Many people in the community think there should be no autoref/deref, I think that a modern language should handle it.

It's just logical. One wouldn't expect the reference to be automatically taken from reference to reference and so on, it's illogical.. just a simple reference is fine, but with more than 1?

In 2025+ I don't want to remember, do I have to pass Config or &Config etc.

For that case we could simply use the autocomplete of any editor or check the documentation of the function that is receiving the value of Config/&Config, as simple as that.

But indeed maybe it should only be done for &Foo, but not for &&&&&Foo, which is too low level and rare.

Exactly! It makes no logic to apply the autoref/deref with more than one reference. Doing it with just one is fine and perfect.


@zeozeozeo

what about just &&Foo? having references to references is pretty common

Although it is true that it is a common case, it is best to leave it explicitly, to avoid entanglements and confusion.

@felipensp felipensp marked this pull request as ready for review November 5, 2024 09:47
@spytheman
Copy link
Member

spytheman commented Nov 5, 2024

All the intermediate temporary variables here, are generated on the stack, in cgen, thus the checker knows nothing about them.

If the produced higher level pointer, is then stored in a struct or returned back, or otherwise escape the function, you will get a very hard to debug situation, that can not be checked at compile time, and is hard to catch at runtime too (if you are lucky, you will get an invalid memory access soon ... if not, you will get silent reading/writing of garbage data, that could be very far from where it becomes visible) ...

Imho strict counting of pointer levels, without any guessing, and auto (de)referencing enshitification, that has errors for mismatches, is superior in most ways (except that it is perhaps a bit more verbose):

  • it is easier to understand for people
  • it is easier to implement in the compiler without special cases
  • it is easier to make meaningful and helpful error checks for it

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.

cgen doesn't create temporaries for arguments with multiple references
5 participants