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

fix compiler crash with .global initializers #1472

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Dec 5, 2024

Summary

Fix the compiler crashing for initializers of .global variables where
the expression contains locals or temporaries requiring destruction.

Fixes #1470.

Details

MIR generation for .global init fragments is done via
mirgen.generateAssignment, which didn't push a scope prior to
translation. This caused any call to mirgen_blocks.register to crash
the compiler, as the scope/block list was empty.

A scope is now pushed prior to translating the expression, fixing the
crash. In addition to fixing the crash, this slightly changes
behaviour: unscoped temporaries/locals created as part of a .global
initializer are now destroyed immediately after the initialization,
instead of at the end of the module's pre-init procedure. Destruction
order w.r.t. .global initializers is unspecified, so this change
shouldn't be a problem.

Summary
=======

Fix the compiler crashing for initializers of `.global` variables where
the expression contains locals or temporaries requiring destruction.

Details
=======

MIR generation for `.global` init fragments is done via
`mirgen.generateAssignment`, which didn't push a scope prior to
translation. This caused any call to `mirgen_blocks.register` to crash
the compiler, as the scope/block list was empty.

A scope is now pushed prior to translating the expression, fixing the
crash. In addition to fixing the crash, this slightly changes
behaviour: unscoped temporaries/locals created as part of a `.global`
initializer are now destroyed immediately after the initialization,
instead of at the end of the module's pre-init procedure. Destruction
order w.r.t. `.global` initializers is unspecified, so this change
shouldn't be a problem.
@zerbina zerbina added bug Something isn't working compiler/backend Related to backend system of the compiler labels Dec 5, 2024
@zerbina zerbina added this to the MIR phase milestone Dec 5, 2024
@alaviss
Copy link
Contributor

alaviss commented Dec 10, 2024

@zerbina can you rebase this?

@zerbina
Copy link
Collaborator Author

zerbina commented Dec 10, 2024

/merge

Copy link

Merge requested by: @zerbina

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

@chore-runner chore-runner bot added this pull request to the merge queue Dec 10, 2024
Merged via the queue into nim-works:devel with commit c189449 Dec 11, 2024
35 checks passed
@zerbina zerbina deleted the mirgen-fix-missing-scope branch January 6, 2025 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/backend Related to backend system of the compiler
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

internal error: 'index out of bounds' in Nimskull with some vtable implementation that uses closures
3 participants