MichalStrehovsky / iltrim

MIT License
9 stars 1 forks source link

Add support for TypeSpec signatures #38

Closed MichalStrehovsky closed 2 years ago

MichalStrehovsky commented 2 years ago

(Can be worked on once #33 is merged.)

To repro, introduce a generic type in the app (class Foo<T> { }) and then make another type derive from it class Bar : Foo<SomeUnusedType> { }. Then reference Bar from somewhere.

To fix: We're currently copying the signature blob that specifies the generic arguments verbatim, without analyzing or rewriting it:

https://github.com/MichalStrehovsky/iltrim/blob/f416567a7d888da3243203dcf8394e1519ddb8d1/src/coreclr/tools/ILTrim/ILTrim/DependencyAnalysis/TokenBased/TypeSpecificationNode.cs#L27-L40

We'll want to do something similar to what was done to method signatures in https://github.com/MichalStrehovsky/iltrim/pull/33/commits/f632a122539cb89166a4f13761639d8272c7772b so that the blob is interpreted.

The blob pretty much contains just a signature that RewriteType and AnalyzeType already understands, so this is a lot less work than the method signatures referenced above.

LakshanF commented 2 years ago

I'm running into issues reading the typeSpec.Signature via the EcmaSignatureAnalyzer, specifically, the blob's byte[] doesn't seem to be correct (the parameter count is wrong and the SignatureTypeCode doesn't look correct as well.

I'm using the following in TypeSpec GetStaticDependencies,

            DependencyList dependencies = new DependencyList();

            EcmaSignatureAnalyzer.AnalyzeMethodSignature(
                _module,
                _module.MetadataReader.GetBlobReader(typeSpec.Signature),
                factory,
                dependencies);

            return dependencies;
sbomer commented 2 years ago

AnalyzeMethodSignature reads MethodDefSig or I think MethodRefSig, but not TypeSpec. You'll need to add a new helper for the typespec signatures (not sure how much can be shared with other signature parsers) to match the signature format for TypeSpecs (see II.23.2.14 TypeSpec of ECMA-335).

For example one difference is: it looks like the TypeSpecs are unique in that they don't have a header as understood by SRM: https://github.com/MichalStrehovsky/iltrim/blob/f6ed887aee37fb4d79aad9079481f9beaaeb0f1d/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Signatures/SignatureHeader.cs#L13-L14 so I don't think you want to call ReadSignatureHeader for TypeSpecs signatures.