Skip to content

Non-sense cyclic initialization #5149

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

Closed
paolodelfino opened this issue May 11, 2025 · 9 comments
Closed

Non-sense cyclic initialization #5149

paolodelfino opened this issue May 11, 2025 · 9 comments

Comments

@paolodelfino
Copy link

Context

        Odin:    dev-2025-04-nightly:d9f990d
        Backend: LLVM 20.1.0
View :: struct {
	widget_make:    proc(),
	widget_handler: proc(),
}

// Top level statement
view_hex := View {
	widget_make = proc() {
               a := view_hex.widget_handler
	},
}

Expected Behavior

view_hex is a pointer and therefore it's an easy relative memory read, except if we're then breaking the paradigm (this is my doubt).

Current Behavior

 Error: Cyclic initialization of 'view_hex' 
        view_hex := View { 
        ^ 
@Kelimion
Copy link
Member

Kelimion commented May 11, 2025

This isn't a cycle, but it should report that view_hex is undeclared, because within the anonymous proc you're assigning to widget_make, it is. view_hex isn't a global it has any knowledge of.

And indeed, I get the expected errors when I compile with odin build . -vet -vet-tabs -strict-style -vet-style -warnings-as-errors -disallow-do. How did you compile it for you to get that cycle error?

package scratch

main :: proc() {
	View :: struct {
		widget_make:    proc(),
		widget_handler: proc(),
	}

	// Top level statement
	view_hex := View {
		widget_make = proc() {
			a := view_hex.widget_handler
		},
	}
}
W:/Scratch/scratch.odin(10:2) Error: 'view_hex' declared but not used 
	view_hex := View { 
	^ 
W:/Scratch/scratch.odin(12:4) Error: 'a' declared but not used 
	a := view_hex.widget_handler 
	^ 
W:/Scratch/scratch.odin(12:9) Error: Undeclared name: view_hex 
	a := view_hex.widget_handler 

And even with just odin build ., you get the following:

W:\Scratch>odin build .
W:/Scratch/scratch.odin(12:9) Error: Undeclared name: view_hex
        a := view_hex.widget_handler
             ^~~~~~~^

@Kelimion
Copy link
Member

Kelimion commented May 11, 2025

If tried this way, it is a cycle, and the error is correct.

package scratch

View :: struct {
	widget_make:    proc(),
	widget_handler: proc(),
}

// Top level statement
view_hex := View {
	widget_make = proc() {
		a := view_hex.widget_handler
	},
}

main :: proc() {

}

@paolodelfino
Copy link
Author

Wait, first of all, I talked about top level statement, so your first example is irrelevant. Second, why would cycle initialization be a correct error here if view_hex is a pointer looking at the assembly and you can do a relative memory read? The only way this is a correct error is if it breaks the paradigm, but at that point the question is if the paradigm is worth it, so you should answer to what are you exchanging for.
@gingerBill can you make a take on this.

@Kelimion
Copy link
Member

Second, why would cycle initialization be a correct error here if view_hex is a pointer looking at the assembly and you can do a relative memory read?

You almost answered your own question there. But relative to what? You're pointing at the thing you're trying to initialize with the thing you're trying to initialize, so you can then read something slightly off to the side of it.

@Kelimion
Copy link
Member

It could conceivably have a clearer error message, like "Error: Trying to intialize a member of view_hex with an address relative to itself.", but that's another way of saying it's a cycle. Nevertheless, I'll reopen it to see if we can massage the messaging.

@Kelimion Kelimion reopened this May 11, 2025
@Kelimion
Copy link
Member

Kelimion commented May 11, 2025

It can be done in two steps, and this is no longer a cycle.

package scratch

import "core:fmt"

View :: struct {
	widget_make:    proc(),
	widget_handler: proc(),
}

// Top level statement
view_hex: View

@(init)
init_view_hex :: proc() {
	view_hex.widget_make = proc() {
		a := view_hex.widget_handler
		fmt.printfln("hello from widget_make: %p", a)
	}
}

main :: proc() {
	fmt.printfln("view_hex: %#v", view_hex)
	view_hex.widget_make()
}
view_hex: View{
	widget_make = proc() @ 0x7FF685613ED0,
	widget_handler = nil,
}
hello from widget_make: nil

@paolodelfino
Copy link
Author

paolodelfino commented May 11, 2025

There is some sort of misunderstanding here.
view_hex is a global variable, therefore it's in memory, therefore when you reference it, you always reference an address. So let's say that it's 0x1000, then probably view_hex.widget_handler will be at 0x1000 + 16. So, it's not initializing it with itself, because you can just do a relative memory read (what I was talking before), which is 0x1000 + 16, when you need to access the value of view_hex.widget_handler. The only thing you immediately need is the address of the variable itself (in this case 0x1000), which you can gather.
So, probably the only way this couldn't or shouldn't work would be due to the language's paradigm (which I also already mentioned).

@Kelimion
Copy link
Member

I've been programming in assembly for just shy of 40 years, thanks. All that aside, it's still a cycle in Odin for the reasons I've explained: You're initializing something with a reference to itself, which you need so you can take a relative address of it.

That you can do this in asm, and even codegen the Odin equivalent in theory, doesn't make it valid Odin.

@paolodelfino
Copy link
Author

So, yeah, you're just basically saying it's a problem raised by the language's paradigm, which can as it cannot be fine (this is another discussion). As simple as that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants