Skip to content

Conversation

@MichalPetryka
Copy link
Contributor

Summary

Those are documented to push managed pointers on the stack in the ECMA 335 and the runtime implements them as such.

Those are documented to push managed pointers on the stack in the ECMA 335 and the runtime implements them as such.
@MichalPetryka MichalPetryka requested a review from a team as a code owner January 25, 2024 22:52
@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-System.Reflection.Emit labels Jan 25, 2024
@learn-build-service-prod
Copy link

Learn Build status updates of commit 8117734:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Reflection.Emit/OpCodes.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

Copy link
Contributor

@reflectronic reflectronic left a comment

Choose a reason for hiding this comment

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

conv.u does not convert managed pointers to transient pointers (type *), so this patch is incorrect. The transient pointers concept was deleted from the standard recently in dotnet/runtime#86622, so any reference to transient pointers should be deleted. There are many more references elsewhere in this file.

@gewarren
Copy link
Contributor

gewarren commented Mar 6, 2024

Tagging @dotnet/area-system-reflection-emit

@steveharter
Copy link
Contributor

PTAL @AaronRobinsonMSFT

@learn-build-service-prod
Copy link

Learn Build status updates of commit f1e1045:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Reflection.Emit/OpCodes.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@AaronRobinsonMSFT
Copy link
Member

/cc @gewarren

@gewarren
Copy link
Contributor

@AaronRobinsonMSFT Is this how it should render?

image

@AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT Is this how it should render?

image

Definitely not. Let me try again. :)

@learn-build-service-prod
Copy link

Learn Build status updates of commit a9f13cd:

✅ Validation status: passed

File Status Preview URL Details
xml/System.Reflection.Emit/OpCodes.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@AaronRobinsonMSFT
Copy link
Member

@gewarren Should be fixed now.

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

LGTM

@gewarren gewarren merged commit a31d808 into dotnet:main Mar 21, 2024
jkotas added a commit that referenced this pull request Jul 5, 2025
Apply #9574 to ldloca.s.

---------

Co-authored-by: Jan Kotas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Reflection.Emit community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants