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

Procedure declaration in defer block crashes compiler #1942

Open
Despacito696969 opened this issue Aug 10, 2022 · 5 comments
Open

Procedure declaration in defer block crashes compiler #1942

Despacito696969 opened this issue Aug 10, 2022 · 5 comments
Labels
replicated We were able to replicate the bug. stale

Comments

@Despacito696969
Copy link
Contributor

Compile following with odin build .

package main

main :: proc()
{
   defer
   {
      foo :: proc(x: int) {return x}
   }
}

You should get:

D:\a\Odin\Odin\src\llvm_backend_stmt.cpp(42): Assertion Failure: `e != nullptr`

Context

Odin: dev-2022-08-nightly:4c3281b3
OS:   Windows 10 Professional (version: 21H2), build 19044.1826
CPU:  Intel(R) Core(TM) i5-8600K CPU @ 3.60GHz
RAM:  16330 MiB
@Kelimion Kelimion added the replicated We were able to replicate the bug. label Aug 10, 2022
@Kelimion
Copy link
Member

Also interesting that this gives you undeclared name: foo:

package main

main :: proc()
{
   defer
   {
      foo :: proc(x: int) {return x}
      foo(1)
   }
}

@gingerBill
Copy link
Member

This begs the question, is this something we should allow in the first place?

defer really is an edge in many ways (e.g. it disallows return within it for loads of sanity reasons (not technical))

@Kelimion
Copy link
Member

Kelimion commented Aug 10, 2022

This begs the question, is this something we should allow in the first place?

I'm inclined to say no. There's no good reason foo couldn't be a child proc of main or even its own top-level proc.

If extrapolated to a larger codebase, it would also be more maintainable to enforce defer hosting only simple expressions and statements, not procedure definitions.

defer blocks by design are meant to be a compact "clean up what we did just above" kind of block. If that cleanup is complicated, put it in its own procedure and call that, e.g. defer destroy_thing(thing).

@awwdev
Copy link
Member

awwdev commented Aug 11, 2022

This begs the question, is this something we should allow in the first place?

I think many users, myself included, would have expected constant declarations :: inside a defer block to work. Variable declarations are also allowed inside a defer block. So the user thinks that they are allowed to write normal code within defer as if it were just a simple copy of contents to the end of the containing scope. A more restrictive defer means that the language rules become more complex and less intuitive.

But @Kelimion's arguments also make a lot of sense to me.

@gingerBill
Copy link
Member

The issue isn't :: but rather procedure declarations with defer (I know what the bug is and how to fix it).

I'm wondering what the best approach to this is. Understand that this is not a technical problem but a design problem. You could easily have as many constant value and type declarations as you want within a defer.

@github-actions github-actions bot added the stale label Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
replicated We were able to replicate the bug. stale
Projects
None yet
Development

No branches or pull requests

4 participants