Skip to content

Support nint/intptr conversion on c# 11 and up #76181

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

Merged
merged 4 commits into from
Dec 3, 2024

Conversation

CyrusNajmabadi
Copy link
Member

Fixes #74973

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner November 30, 2024 19:27
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 30, 2024
@CyrusNajmabadi CyrusNajmabadi changed the title Support nit/intptr conversion on c# 11 and up Support nint/intptr conversion on c# 11 and up Nov 30, 2024
public async Task TestNint1_WithNumericIntPtr_CSharp11_NoRuntimeSupport()
{
await TestInRegularAndScript1Async("""
<Workspace>
Copy link
Contributor

Choose a reason for hiding this comment

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

For my benefit, why does this use the markup/workspace string format rather than just specifying the string as most of the tests do in this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

because it needs to specify the set of references/runtime the project is configured against. For the legacy test framework (the one that is not using the VerifyCS pattern) this can only be done with this <Workspace> format. For the modern test framework, you can do ReferenceAssemblies = ReferencesAssemblies.Net.Net80 and stuff like that.

Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

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

:shipit:

@CyrusNajmabadi CyrusNajmabadi merged commit df02af6 into dotnet:main Dec 3, 2024
25 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the nintEquivalence branch December 3, 2024 00:53
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Dec 3, 2024
@tannergooding
Copy link
Member

@CyrusNajmabadi, I don't believe this is correct.

The C# 9 feature was to introduce nint as a language keyword, but the behavior of nint and IntPtr could differ and as such they are not considered equivalent.

In C# 11, it was changed so that if the runtime feature exists then the language can assume nint and IntPtr have identical behavior and that it is safe to treat them as strict aliases of eachother.

Accordingly, even if a user manually targets C# 11 then on a runtime (such as .NET Framework) without the corresponding runtime feature define, then nint.M() and IntPtr.M() can differ in behavior and this can lead to quirks and other bugs.

@tannergooding
Copy link
Member

This is covered in the feature speclet: https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/numeric-intptr.md

Which explicitly states that the new behavior is only triggered by existence of the System.Runtime.CompilerServices.RuntimeFeature.NumericIntPt runtime feature flag.

@CyrusNajmabadi
Copy link
Member Author

Will fix in followup. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use language keywords does not work for nint and nuint when targeting net4.8.1 using LangVersion 9.0 or higher
4 participants