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

[mono][interp] Fix GetType called on ptr constrained to Nullable` #61020

Merged
merged 2 commits into from
Nov 4, 2021

Conversation

BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Oct 29, 2021

We were statically optimizing this call to return the actual constrained class type, which is incorrect for nullables, because boxing of a nullable (as part of the constrained call) actually creates an object with the type of the nullable's value (or null if there is no value).

Fixes #61007

@ghost
Copy link

ghost commented Oct 29, 2021

Tagging subscribers to this area: @BrzVlad
See info in area-owners.md if you want to be subscribed.

Issue Details

We were statically optimizing this call to return the actual constrained class type, which is incorrect for nullables, because boxing of a nullable (as part of the constrained call) actually creates an object with the type of the nullable's value (or null if there is no value).

Fixes #61007

Author: BrzVlad
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@lewing
Copy link
Member

lewing commented Oct 29, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

@radical
Copy link
Member

radical commented Oct 30, 2021

This needs a test.

@BrzVlad
Copy link
Member Author

BrzVlad commented Oct 30, 2021

@radical Any recommendation where ? In the runtime tests ?

@radical
Copy link
Member

radical commented Oct 31, 2021

@radical Any recommendation where ? In the runtime tests ?

Adding in runtime tests makes sense.

We were statically optimizing this call to return the actual constrained class type, which is incorrect for nullables, because boxing of a nullable (as part of the constrained call) actually creates an object with the type of the nullable's value (or null if there is no value).
@BrzVlad
Copy link
Member Author

BrzVlad commented Nov 1, 2021

  1. Added a test case. Set the priority to 0 for now so it gets to run on this PR on all configurations. I think I should leave it with priority 1, although I have no idea what are the guidelines around test priorities.
  2. Changed the fix so it is a bit more reasonable. With the previous fix we were hitting some confusing code paths. Also this fix prevents addition of a cknull and some redundant inlining.

@lewing
Copy link
Member

lewing commented Nov 2, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2021

@lewing lewing merged commit c02a37c into dotnet:main Nov 4, 2021
BrzVlad added a commit to BrzVlad/runtime that referenced this pull request Nov 8, 2021
…tnet#61020)

* [interp] Fix GetType called on ptr constrained to Nullable`

We were statically optimizing this call to return the actual constrained class type, which is incorrect for nullables, because boxing of a nullable (as part of the constrained call) actually creates an object with the type of the nullable's value (or null if there is no value).

* Add test for GetType call on ptr constrained to nullable
Anipik pushed a commit that referenced this pull request Nov 12, 2021
…1020) (#61305)

* [interp] Fix GetType called on ptr constrained to Nullable`

We were statically optimizing this call to return the actual constrained class type, which is incorrect for nullables, because boxing of a nullable (as part of the constrained call) actually creates an object with the type of the nullable's value (or null if there is no value).

* Add test for GetType call on ptr constrained to nullable
@ghost ghost locked as resolved and limited conversation to collaborators Dec 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.NET 6.0 blazor wasm project reports an incorrect type
5 participants