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.

Sign up for GitHub

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

Jump to bottom

Emit custom modifiers into native layout metadata #91381

Open
MichalStrehovsky opened this issue Aug 31, 2023 · 1 comment
Open

Emit custom modifiers into native layout metadata #91381

MichalStrehovsky opened this issue Aug 31, 2023 · 1 comment
Labels
area-NativeAOT-coreclr
Milestone
10.0.0

Comments

@MichalStrehovsky
Copy link
Member

When we started generating custom modifiers into metadata format to support new function pointer APIs in #85504, we should have also added it to native layout format. We currently have a mismatch. It's likely generic virtual dispatch and LDTOKEN is going to be wrong if we're in a situation where two method signatures only differ by modifiers.
@MichalStrehovsky MichalStrehovsky added this to the 9.0.0 milestone Aug 31, 2023
@ghost
Copy link

ghost commented Aug 31, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

When we started generating custom modifiers into metadata format to support new function pointer APIs in #85504, we should have also added it to native layout format. We currently have a mismatch. It's likely generic virtual dispatch and LDTOKEN is going to be wrong if we're in a situation where two method signatures only differ by modifiers.

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr
Milestone: 9.0.0

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Aug 31, 2023
@MichalStrehovsky
Fix LDTOKEN of methods that have modifiers
66fb2be 
When we started generating custom modifiers into metadata format to support new function pointer APIs in dotnet#85504, we should have also added it to native layout format. We currently have a mismatch. This is a low risk bugfix to ignore modifiers on the metadata side. We'll want to do a full fix to actually emit and compare this. Tracked in a .NET 9 bug at dotnet#91381.

No regression test because I spent too much time being puzzled at why https://github.com/Handlebars-Net/Handlebars.Net/blob/50614fd844e5360eb10e76154aa74da4d7bf12ce/source/Handlebars/Helpers/IHelperDescriptor.cs#L13 is generated as a custom modifier (`[in] !TOptions& modreq([netstandard]System.Runtime.InteropServices.InAttribute) options`) whereas if I do it, I get `[in] !T& 'value'` with a custom attribute. We'll want to write a proper set of tests with ambiguities for the bug I opened anyway.
@MichalStrehovsky MichalStrehovsky mentioned this issue Aug 31, 2023
Merged
@MichalStrehovsky MichalStrehovsky modified the milestone: 9.0.0 Aug 31, 2023
@ghost ghost added untriaged New issue has not been triaged by the area owner and removed untriaged New issue has not been triaged by the area owner labels Aug 31, 2023
MichalStrehovsky added a commit that referenced this issue Sep 1, 2023
@MichalStrehovsky
Fix LDTOKEN of methods that have modifiers (#91382)
43f3e60 
When we started generating custom modifiers into metadata format to support new function pointer APIs in #85504, we should have also added it to native layout format. We currently have a mismatch. This is a low risk bugfix to ignore modifiers on the metadata side. We'll want to do a full fix to actually emit and compare this. Tracked in a .NET 9 bug at #91381.

No regression test because I spent too much time being puzzled at why https://github.com/Handlebars-Net/Handlebars.Net/blob/50614fd844e5360eb10e76154aa74da4d7bf12ce/source/Handlebars/Helpers/IHelperDescriptor.cs#L13 is generated as a custom modifier (`[in] !TOptions& modreq([netstandard]System.Runtime.InteropServices.InAttribute) options`) whereas if I do it, I get `[in] !T& 'value'` with a custom attribute. We'll want to write a proper set of tests with ambiguities for the bug I opened anyway.
github-actions bot pushed a commit that referenced this issue Sep 1, 2023
@MichalStrehovsky
Fix LDTOKEN of methods that have modifiers
b872a63 
When we started generating custom modifiers into metadata format to support new function pointer APIs in #85504, we should have also added it to native layout format. We currently have a mismatch. This is a low risk bugfix to ignore modifiers on the metadata side. We'll want to do a full fix to actually emit and compare this. Tracked in a .NET 9 bug at #91381.

No regression test because I spent too much time being puzzled at why https://github.com/Handlebars-Net/Handlebars.Net/blob/50614fd844e5360eb10e76154aa74da4d7bf12ce/source/Handlebars/Helpers/IHelperDescriptor.cs#L13 is generated as a custom modifier (`[in] !TOptions& modreq([netstandard]System.Runtime.InteropServices.InAttribute) options`) whereas if I do it, I get `[in] !T& 'value'` with a custom attribute. We'll want to write a proper set of tests with ambiguities for the bug I opened anyway.
carlossanlop pushed a commit that referenced this issue Sep 1, 2023
@github-actions @MichalStrehovsky
Fix LDTOKEN of methods that have modifiers (#91439)
62b57fc 
When we started generating custom modifiers into metadata format to support new function pointer APIs in #85504, we should have also added it to native layout format. We currently have a mismatch. This is a low risk bugfix to ignore modifiers on the metadata side. We'll want to do a full fix to actually emit and compare this. Tracked in a .NET 9 bug at #91381.

No regression test because I spent too much time being puzzled at why https://github.com/Handlebars-Net/Handlebars.Net/blob/50614fd844e5360eb10e76154aa74da4d7bf12ce/source/Handlebars/Helpers/IHelperDescriptor.cs#L13 is generated as a custom modifier (`[in] !TOptions& modreq([netstandard]System.Runtime.InteropServices.InAttribute) options`) whereas if I do it, I get `[in] !T& 'value'` with a custom attribute. We'll want to write a proper set of tests with ambiguities for the bug I opened anyway.

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
@MichalStrehovsky MichalStrehovsky modified the milestones: 9.0.0, 10.0.0 Jun 28, 2024
@agocke agocke added this to AppModel Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-NativeAOT-coreclr
Projects
AppModel
Status: No status
Milestone
10.0.0
Development

No branches or pull requests
1 participant
@MichalStrehovsky