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

Make MetadataDisplayFormat public #72844

Open
tmat opened this issue Apr 2, 2024 · 2 comments
Open

Make MetadataDisplayFormat public #72844

tmat opened this issue Apr 2, 2024 · 2 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Milestone

Comments

@tmat
Copy link
Member

tmat commented Apr 2, 2024

Background and Motivation

After a compilation is emitted and the resulting image is loaded, we need to find a type in the loaded Assembly that corresponds to a source INamedTypeSymbol defined in the compilation.
 

Proposed API

namespace Microsoft.CodeAnalysis;

public class SymbolDisplayFormat
{
+   SymbolDisplayFormat MetadataDisplayFormat { get; }
}

Implemented like so:

private static readonly SymbolDisplayFormat s_metadataDisplayFormat =
SymbolDisplayFormat.QualifiedNameArityFormat.AddCompilerInternalOptions(SymbolDisplayCompilerInternalOptions.UsePlusForNestedTypes);

Usage Examples

var qualifiedTypeName = typeSymbol.ToDisplayString(SymbolDisplayFormat.MetadataDisplayFormat);
var type = Assembly.GetType(qualifiedTypeName);

Alternative Designs

Risks

@tmat tmat added Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request labels Apr 2, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 2, 2024
@tmat tmat added the api-ready-for-review API is ready for review, it is NOT ready for implementation label Apr 2, 2024
@jaredpar jaredpar added this to the 17.11 milestone Apr 5, 2024
@333fred
Copy link
Member

333fred commented Apr 11, 2024

API Review

  • Rather than exposing a new MetadataDisplayFormat, can we just expose UsePlusForNestedTypes option?
  • Let users then create the format they need
  • What part of metadata format does this cover? Is it class declaration? Is it call syntax with substituted generic types?
    • If we want to have a premade format for this scenario, we should be very precise as to what is supported and what is not.

Conclusion: Needs work. Exposing a UsePlusForNestedTypes option seems reasonable by itself. A format would then need to be very precise as to what it supports and where.

@333fred 333fred added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 11, 2024
@tmat
Copy link
Member Author

tmat commented Apr 12, 2024

My bad, I didn't notice that s_metadataDisplayFormat is only used by tests and is not the full implementation of fully qualified name.

The intention was to allow the code written in the usage example - that is, given a ITypeSymbol return a fully qualified metadata name in the format that System.Reflection parses to load a type. See https://github.com/dotnet/runtime/blob/main/src/libraries/Common/src/System/Reflection/TypeNameParser.cs.

This format is also used in ECMA-335 when serializing the value of System.Type for custom attribute blob. This is implemented by the compiler here: https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/PEWriter/TypeNameSerializer.cs

Examples:

typeof(X<int*[]>.Y<string>[,,]))
X`1+Y`1[[System.Int32*[], System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.String, System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]][,,]
typeof(X<>.Y<>)
X`1+Y`1

@jcouv jcouv removed the untriaged Issues and PRs which have not yet been triaged by a lead label Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Projects
None yet
Development

No branches or pull requests

4 participants