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

[API contract regression] SymbolDisplay.ToDisplayString(IntPtrTypeSymbol, FullnameFormat) returns nint instead of System.IntPtr with .NET 8.0 SDK #76895

Open
ceztko opened this issue Jan 23, 2025 · 4 comments
Assignees
Labels
api-ready-for-review API is ready for review, it is NOT ready for implementation Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature - Native Int
Milestone

Comments

@ceztko
Copy link

ceztko commented Jan 23, 2025

Description:

In a simple compilation SymbolDisplay.ToDisplayString(IntPtrTypeSymbol, FullnameFormat), with IntPtrTypeSymbol the type symbol for System.IntPtr and FullnameFormat defined below, returns nint instead of System.IntPtr when compiled depending on .NET 8.0 SDK, despite no SymbolDisplayMiscellaneousOptions.UseSpecialTypes is being used. If the same source is compiled depending on .NET 6.0 SDK the returned value is System.IntPtr, as expected.

FullnameFormat is defined as:

static readonly SymbolDisplayFormat FullnameFormat = new SymbolDisplayFormat(
    globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.OmittedAsContaining,
    typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
    localOptions: SymbolDisplayLocalOptions.IncludeType,
    genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters
    | SymbolDisplayGenericsOptions.IncludeVariance,
    memberOptions:
        SymbolDisplayMemberOptions.IncludeContainingType |
        SymbolDisplayMemberOptions.IncludeExplicitInterface,
    miscellaneousOptions:
        SymbolDisplayMiscellaneousOptions.UseErrorTypeSymbolName |
        SymbolDisplayMiscellaneousOptions.ExpandNullable
        // NOTE: SymbolDisplayMiscellaneousOptions.UseSpecialTypes is not used!

Version Used:

Microsoft.CodeAnalysis.CSharp 4.12

Steps to Reproduce:

  1. Decompress IntPtrTest.zip and launch the IntPtrTest.sln solution
  2. Run the Net6SystemIntPtr project, it will print net6.0 dependency: IntPtr fullname is System.IntPtr;
  3. Run the Net8nint project, it will print net8.0 dependency: IntPtr fullname is nint.

Expected Behavior:

I understand that using .NET 8.0 SDK nint is now an alias for System.IntPtr (or the other way round). Despite this SymbolDisplayMiscellaneousOptions.UseSpecialTypes is documented in this way:

/// <summary>
/// Uses keywords for predefined types. 
/// For example, "int" instead of "System.Int32" in C#
/// or "Integer" instead of "System.Integer" in Visual Basic.
/// </summary>
UseSpecialTypes = 1 << 0,

Similarly I expect nint to be a special type, and not having used this flag I expect SymbolDisplay.ToDisplayString(IntPtrTypeSymbol, FullnameFormat) to return System.IntPtr.

Actual Behavior:

Depending on the dependent SDK used for the compilation,SymbolDisplay.ToDisplayString returns a different value for the ubiquitous System.IntPtr type, which is burden when maintaining analyzers that will process code that can be dependent on a unpredictable SDK.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 23, 2025
@CyrusNajmabadi
Copy link
Member

which is burden when maintaining analyzers that will process code that can be dependent on a unpredictable SDK.

Could you clarify this. I would not expect analyzers to need the display string, or find themselves in a difficult to maintain state due to the display string of a particular symbol. Thanks!

@ceztko
Copy link
Author

ceztko commented Jan 24, 2025

Could you clarify this. I would not expect analyzers to need the display string, or find themselves in a difficult to maintain state due to the display string of a particular symbol. Thanks!

Sure thing. I mentioned "analyzers", but I really meant anything using Roslyn. I'm maintaining a transpiler for a subset of C# to different languages/frameworks. I'm often in the need of checking against my custom list of "special types". While I understand it may be unreliable in some circumstances, I'm using the SymbolDisplayFormat I defined above to create a string identifier for the type, and I'm actively checking against it in multiple places. Knowing that the approach is not 100% correct (for example the same qualified type name may be defined in multiple assemblies), it works well at my conditions so I would focus on the API contract for SymbolDisplayMiscellaneousOptions.UseSpecialTypes which is enough clear. The situation now is that with the SymbolDisplayFormat above all older "special" type symbols like void, int, float, etc. are still correctly displayed respectively as System.Void, System.Int32, System.Single, etc., while IntPtr, nint or whatever maps to System.IntPtr now is displayed unconditionally nint (but only when targeting >= .NET 8.0) which is odd/slack. My guess here is that for the same reason using SymbolDisplay.ToDisplayString is not the most reliable way to create a identifier for types, a fix here could be done without creating too much concern (handling nuint/UIntPtr as well).

@ceztko
Copy link
Author

ceztko commented Jan 25, 2025

I did some research and debugging session with the roslyn compiler myself. The branch conditional where Roslyn chooses to display nint instead of System.IntPtr is here. In the branch conditional,SymbolDisplayCompilerInternalOptions.UseNativeIntegerUnderlyingType would also allow to display exactly what I want (System.IntPtr instead of nint). SymbolDisplayCompilerInternalOptions is an internal enum and UseNativeIntegerUnderlyingType is a really little used enum value inside Roslyn code (I could find only 5 references). From a brief look it appears it's used just in unit tests. As I wild guess it could be just removed, and the tests could just ensure the already mentioned public SymbolDisplayMiscellaneousOptions.UseSpecialTypes is not used when they want to read the underlying struct integer type.

@jaredpar jaredpar added Feature - Native Int and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 5, 2025
@jaredpar jaredpar added this to the 17.14 milestone Feb 5, 2025
@cston
Copy link
Member

cston commented Feb 5, 2025

Marking for API review.

As suggested in the issue description, we should consider using SymbolDisplayMiscellaneousOptions.UseSpecialTypes to control display of nint and nuint. If UseSpecialTypes is set, the types should be displayed as nint and nuint; otherwise as System.IntPtr and System.UIntPtr.

This is a breaking change, however. Specifically, for existing code that does not set UseSpecialTypes, the types will no longer be displayed as nint and nuint.

@cston cston added Concept-API This issue involves adding, removing, clarification, or modification of an API. api-ready-for-review API is ready for review, it is NOT ready for implementation labels Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-ready-for-review API is ready for review, it is NOT ready for implementation Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature - Native Int
Projects
None yet
Development

No branches or pull requests

4 participants