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 crash when using sizeof, instance_sizeof, or offsetof as a type arg #12577

Merged
merged 2 commits into from
Nov 30, 2022

Conversation

keidax
Copy link
Contributor

@keidax keidax commented Oct 9, 2022

This doesn't prevent using sizeof in some cases. Something like this works still:

foo : Int32[sizeof(UInt8)] = Int32.static_array(1)

Poking around in the compiler code for the first time, I couldn't see a way to expand this functionality to also work for other cases. And based on comments on the related issues, maybe it doesn't make sense to do so. So I decided to just fix the ugly crash message. I believe this resolves #8858, while leaving #5427 open for future work.

The small parser change just allows the error message to point to the right part of the line, like

 1 | alias Foo = Array(sizeof(Int32))
                       ^
Error: can't use sizeof(Int32) as a generic type argument

@Blacksmoke16 Blacksmoke16 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels Oct 9, 2022
Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@beta-ziliani beta-ziliani added this to the 1.7.0 milestone Nov 25, 2022
@keidax keidax force-pushed the fix-sizeof-type-arg-crash branch from cb81ab1 to 2ff261b Compare November 27, 2022 14:45
@beta-ziliani
Copy link
Member

Please @keidax do not force-push the branch, as we lost track of what did we review. Thanks!

@keidax
Copy link
Contributor Author

keidax commented Nov 29, 2022

@beta-ziliani I was just rebasing the branch to fix the failing CI, I won't force push in the future

@straight-shoota
Copy link
Member

Merging master is preferred over rebasing because that's more transparent. We squash merge in the end, so it doesn't matter to have merge commits in the branch.

@straight-shoota straight-shoota merged commit 1b93218 into crystal-lang:master Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] using sizeof in an abstract method definition crashes the compiler
4 participants