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: make possible to initialize sumtype with default type (first variant) #22039

Merged
merged 13 commits into from
Aug 16, 2024

Conversation

felipensp
Copy link
Member

Fix #22030

@enghitalo enghitalo changed the title cgen: make possible to initialized sumtype with defaul type (first variant) cgen: make possible to initialized sumtype with default type (first variant) Aug 13, 2024
@spytheman
Copy link
Member

You will have to change vfmt too, so that it would not sort the variants anymore.

@felipensp felipensp marked this pull request as ready for review August 13, 2024 15:08
@felipensp felipensp changed the title cgen: make possible to initialized sumtype with default type (first variant) cgen: make possible to initialize sumtype with default type (first variant) Aug 13, 2024
@felipensp felipensp marked this pull request as draft August 13, 2024 21:39
@StunxFS
Copy link
Contributor

StunxFS commented Aug 15, 2024

And what happens if the first variant of the sumtype is a struct that has some field that is of pointer type? (the compiler should force explicit initialization)

@felipensp
Copy link
Member Author

And what happens if the first variant of the sumtype is a struct that has some field that is of pointer type? (the compiler should force explicit initialization)

In this case it is zero'd initilization, just like {0} was doing before.

Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work.

@spytheman
Copy link
Member

@felipensp about @StunxFS's comment, consider this:

struct Node {
    parent &Node
    left &Node
    right &Node
}
type Sumtype = Node | int

// n := Node{} /* will produce a compiler error, because the reference fields will be nil pointers */

s := Sumtype{} // currently, the compiler does not care, even though the node that will be created by default will be the same as `n` above
dump(s)

producing (without any compiler messages):

#0 02:36:52 ^ fix_sumtype_init /v/oo>v run ss.v
[ss.v:11] s: Sumtype(Node{
    parent: &nil
    left: &nil
    right: &nil
})

@spytheman
Copy link
Member

spytheman commented Aug 15, 2024

i.e. the compiler currently acts on s := Sumtype{}, as if I had written:

struct Node {
    parent &Node = unsafe { nil }
    left &Node = unsafe { nil }
    right &Node = unsafe { nil }
}

It is a second order effect. This PR already makes sure, that s itself will be valid, while it was not before (the unknown sumtype value would break matches etc). The default value in the sumtype value though, is something that the compiler normally prevents, but without additional checks, will be a source of puzzling bugs, for people that depend that their struct instances with references are always properly initialized.

@felipensp felipensp marked this pull request as ready for review August 16, 2024 14:23
@spytheman spytheman merged commit 4940f2d into vlang:master Aug 16, 2024
73 checks passed
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.

sum types should have a default 0 value for the first type mentioned in the list of types.
3 participants