Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Wasm: fix for boxing a float32 into Single#7894

Merged
jkotas merged 9 commits intodotnet:masterfrom
yowl:box-single
Dec 24, 2019
Merged

Wasm: fix for boxing a float32 into Single#7894
jkotas merged 9 commits intodotnet:masterfrom
yowl:box-single

Conversation

@yowl
Copy link
Contributor

@yowl yowl commented Nov 28, 2019

Add a test and the fix for :

  IL_0023:  ldc.r4     1.1
  IL_0028:  box        [System.Runtime]System.Single

Which was failing due to using a float64 and not trunking the float64 into a float32 before calling RhBox. Also added code to ensure that type used in the reflection has its type symbol add to the dependencies to prevent missing metadata exception. WIP as contains changes from #7891 so will wait for that before releasing.

@yowl yowl changed the title WIP: Wasm: fix for boxing a float32 into Single Wasm: fix for boxing a float32 into Single Dec 12, 2019
MethodDesc helper = _compilation.TypeSystemContext.GetHelperEntryPoint("LdTokenHelpers", "GetRuntimeTypeHandle");
AddMethodReference(helper);
HandleCall(helper, helper.Signature);
if (ConstructedEETypeNode.CreationAllowed(typeDesc))
Copy link
Member

Choose a reason for hiding this comment

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

This should rather be MaximallyConstructableType that does the right thing for both CreationAllowed and !CreationAllowed cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, have changed to be unconditionally MaximallyConstructableType and removed couple of unused variables.

@@ -3109,16 +3109,15 @@ private void ImportLdToken(int token)
{
var ldtokenValue = _methodIL.GetObject(token);
WellKnownType ldtokenKind;
Copy link
Member

Choose a reason for hiding this comment

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

Delete this variable completely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, done.

@jkotas
Copy link
Member

jkotas commented Dec 24, 2019

Could you please fix the conflict?

# Conflicts:
#	tests/src/Simple/HelloWasm/Program.cs
@yowl
Copy link
Contributor Author

yowl commented Dec 24, 2019

Missed that. Done now.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@jkotas jkotas merged commit a3f2141 into dotnet:master Dec 24, 2019
@yowl yowl deleted the box-single branch December 24, 2019 17:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants