Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add mvid and metadata token API to S.R.TypeExtensions#3896

Merged
nguerrera merged 2 commits into
dotnet:masterfrom
nguerrera:mvid-token
Oct 21, 2015
Merged

Add mvid and metadata token API to S.R.TypeExtensions#3896
nguerrera merged 2 commits into
dotnet:masterfrom
nguerrera:mvid-token

Conversation

@nguerrera
Copy link
Copy Markdown
Contributor

NOTE: Marking as WIP because build and tests will fail without dotnet/coreclr#1781 and #3861

This adds ModuleExtensions and MemberInfoExtensions to the the System.Reflection.TypeExtensions library that provides extension methods to bridge reflection gaps. (It already has PropertyInfoExtensions, etc. etc.)

  • Classic MemberInfo.MetadataToken is re-surfaced as MemberInfo.TryGetMetadataToken(out int token).
  • Class Module.ModuleVersionId is re-surfaced as Module.TryGetModuleVersionId(out Guid mvid)

The TryGet pattern is used to draw attention to the fact these will not be supported everywhere (.NET Native will simply return false).

Update: Changed to HasXxx and GetXxx from TryGet after API review feedback.

Other options such as nullables or using default(T) for failure were considered, but one thing that folks do with these is compare the return value from one instance to another which would give silent false positives. I think the scenarios for using these are limited enough for the more cumbersome signature.

Fix #2375

cc @weshaggard @delmyers

@nguerrera nguerrera added the api-needs-work API needs work before it is approved, it is NOT ready for implementation label Oct 15, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this supposed to be an extension method? I don't see the this keyword.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it didn't affect testing because everything goes through the reference assembly.

I actually left off the 'this' in implementation for things that have an exact instance counterpart in mscorlib (otherwise code looks like infinite recursion, but that doesn't apply here, and in fact on desktop you reference the impl and need to see this new method as an extension.

Will fix.

@nguerrera
Copy link
Copy Markdown
Contributor Author

Changed to account for API review feedback. @KrzysztofCwalina PTAL.

@nguerrera
Copy link
Copy Markdown
Contributor Author

(This will still fail CI as the CoreCLR package needs to be updated.)

@KrzysztofCwalina
Copy link
Copy Markdown
Member

looks good.

@nguerrera nguerrera changed the title [WIP] Add mvid and metadata token API to S.R.TypeExtensions Add mvid and metadata token API to S.R.TypeExtensions Oct 21, 2015
@stephentoub
Copy link
Copy Markdown
Member

@dotnet-bot test this please

@nguerrera
Copy link
Copy Markdown
Contributor Author

I've had to update the test-runtime to include Module.ModuleVersionId, which triggered #4040.

@nguerrera nguerrera closed this Oct 21, 2015
@nguerrera nguerrera reopened this Oct 21, 2015
@nguerrera nguerrera added this to the 1.0.0-rc1 milestone Oct 21, 2015
nguerrera added a commit that referenced this pull request Oct 21, 2015
Add mvid and metadata token API to S.R.TypeExtensions
@nguerrera nguerrera merged commit f154e24 into dotnet:master Oct 21, 2015
@nguerrera nguerrera deleted the mvid-token branch October 21, 2015 21:45
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Add mvid and metadata token API to S.R.TypeExtensions

Commit migrated from dotnet/corefx@f154e24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api-needs-work API needs work before it is approved, it is NOT ready for implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants