Mechanism for getting from metadata symbol to underlying handle or raw metadata token

[nguerrera]

It would be useful to be able to correlate a Roslyn symbol originating in metadata to PDBs, IL, and lower level metadata.

I've experimented with it and I'll submit the following two alternative proposals

1. Expose the handle via a new interface that can be queried

master...nguerrera:metadata-symbol

public interface IMetadataSymbol
{
    System.Reflection.Metadata.Handle MetadataHandle { get; }
}

internal class PENamedTypeSymbol : NamedTypeSymbol, IMetadataSymbol
{
   public System.Reflection.Metadata.Handle MetadataHandle => _handle;
    ..
}

// etc. for all of the PEXxxSymbols for C# and VB.

2. Expose the handle directly from ISymbol

master...nguerrera:metadata-symbol-2

public interface ISymbol
{
    System.Reflection.Metadata.Handle MetadataHandle { get; }
    ...
}

internal class Symbol : ISymbol
{
     public virtual System.Reflection.Metadata.Handle MetadataHandle => default(Handle);
     ...
}

internal class PENamedTypeSymbol : NamedTypeSymbol
{
    public override System.Reflection.Metadata.Handle MetadataHandle => _handle;
    ...
}

// etc. for all of the PEXxxSymbols for C# and VB.

So non-metadata symbols simply return default(Handle) which has IsNil property equal to true.

I started with (1) under the assumption that we could not modify an existing interface, but I have since learned about Roslyn's "internal-implementation-only" policy for symbols. I now think (2) is much better and cleaner and aligns with the recent addition of Metadata properties on IAssemblySymbol and IModuleSymbol.

[tmat]

I think using Handle is better. This API would be used for very specific purpose which won't be that common. Note we already have an API that exposes MethodDefinitionHandle for another specific scenario - EnC: http://source.roslyn.io/#Microsoft.CodeAnalysis/Compilation/Compilation.cs,4b458bab239f0cf9,references

[nguerrera]

I've assigned this to myself and I'll update my branch along whatever unfolds as the design from the discussion here and contribute a PR when the design lands.

@tmat I didn't realize there was already such a dependency. In that case, it's pretty clear we should use Handle and not int. I also wondered if it should be EntityHandle, but that would preclude providing a NamespaceDefinitionHandle from namespace symbols. Today, we don't use that to build namespace symbols, so we wouldn't provide it initially, but it seems wise to allow for wider handles in the future.

What do you think of the IMetadataSymbol approach and definition above? Who else should I tag to look at it?

[tmat]

Re: namespaces -- I think we should move Roslyn over to using namespace handles. Currently it does namespace tree building itself, but it would be good to leverage the impl in the reader (unless we find some reason why it can't be done).

Re: the design of IMetadataSymbol -- @JohnHamby @mattwar @gafter @AnthonyDGreen @dotnet/roslyn-compiler

[AlekseyTs]

VB namespaces are case-insensitive.

[nguerrera]

VB namespace are case-insensitive

Good point. Right now, that is enough to prevent a VB.PENamespaceSymbol from being backed by a single NamespaceDefinitionHandle.

I still think using the base Handle (with the extra byte vs. EntityHandle) is most future-proof -- we could opt to use metadata reader namespace tree for C# only or even add case-insensitive handles to the reader (these handles are synthesized and we could build case-sensitive and insensitive trees on demand there too).

[tmat]

@nguerrera Couldn't we support case-insensitive namespaces in the reader? (not sure if it's reasonable, just throwing it out there).

[nguerrera]

@tmat That's what I meant by "right now". We could add case-insensitive tree and handles if we want. It might clutter the S.R.M API (I'm not sure), but it is certainly possible.

[nguerrera]

cc @michaelcfanning

[nguerrera]

@JohnHamby @mattwar @gafter @AnthonyDGreen @dotnet/roslyn-compiler Any thoughts on this? Should I go ahead and clean up what I have with tests and so forth and submit a PR?

[nguerrera]

I've updated the proposal with a second option that I like better.

[agocke]

I do not like the second option where ISymbol has an added handle. Interfaces should contain only methods which are semantically meaningful for the interface. If a method is not semantically valid for an interface it should be split out to a new interface -- in this case IMetadataSymbol.

I think the IMetadataSymbol proposal is fine.