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

Compiler: only make Pointer(T)#value= stricter for generic arguments #10224

Merged
merged 2 commits into from
Jan 8, 2021

Conversation

asterite
Copy link
Member

@asterite asterite commented Jan 8, 2021

Alternative to #10214 (partially reverts that one)
Fixes #10211

This makes the compiler apply the logic of #10214 to only Pointer(T)#value=, preventing incorrect memory from being assigned. In every other case it will work as it used to work (in a more permissive way).

In a way, this is similar to how assigning Array(Array(Int32)) to Array(Array(String | Int32)) is invalid, but passing the former to a method that restricts it by the latter works. Pointer(T)#value= is like an assignment and it should be treated in a more strict way.

@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels Jan 8, 2021
@asterite asterite added this to the 1.0.0 milestone Jan 8, 2021
@@ -1058,7 +1058,7 @@ class Crystal::Call
args["self"] = MetaVar.new("self", self_type)
end

strict_check = body.is_a?(Primitive) && body.name == "proc_call"
strict_check = body.is_a?(Primitive) && (body.name == "proc_call" || body.name == "pointer_set")
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it make sense to remove this variable as its only used in the conditional which could be replaced by case body.as?(Primitive).try &.name?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to make it clear a strict check is happening.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or maybe yes, I don't know. I don't think it matters much, plus I won't be at the computer for a while

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.

Compiler bug in Array#unshift
3 participants