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.

Sign up for GitHub

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

Jump to bottom

Add TypeName APIs to simplify metadata lookup. #111598

Merged
merged 37 commits into from
Feb 10, 2025
Merged

Add TypeName APIs to simplify metadata lookup. #111598

merged 37 commits into from
Feb 10, 2025

Conversation

teo-tsirpanis
Copy link
Contributor

@teo-tsirpanis teo-tsirpanis commented Jan 20, 2025 •
edited
Loading

Fixes #101774.

@dotnet-issue-labeler dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@dotnet-issue-labeler dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 20, 2025
@jkotas
Copy link
Member

jkotas commented Jan 20, 2025

Could you please delete

runtime/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

Lines 1492 to 1494 in 29013d8

<Compile Include="$(CommonPath)System\Reflection\Metadata\TypeNameHelpers.cs">
<Link>Common\System\Reflection\Metadata\TypeNameHelpers.cs</Link>
</Compile>
and replace the calls to it with this API?

(Other uses of TypeNameHelpers.cs should be delete too once these APIs are available in the LKG runtime.)

@jkotas jkotas requested a review from adamsitnik January 20, 2025 03:02
This was referenced Jan 20, 2025
Open
Open
Open
@teo-tsirpanis teo-tsirpanis requested a review from Copilot January 20, 2025 22:15
Copilot
Copilot AI reviewed Jan 20, 2025
View reviewed changes

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 17 changed files in this pull request and generated no comments.

Files not reviewed (12)
  • src/coreclr/tools/ILVerification/ILVerification.projitems: Language not supported
  • src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj: Language not supported
  • src/coreclr/tools/aot/ILCompiler.TypeSystem/ILCompiler.TypeSystem.csproj: Language not supported
  • src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems: Language not supported
  • src/tools/illink/src/linker/Mono.Linker.csproj: Language not supported
  • src/libraries/Common/src/System/Reflection/Metadata/TypeNameHelpers.cs: Evaluated as low risk
  • src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs: Evaluated as low risk
  • src/tools/illink/src/linker/Linker/TypeNameResolver.cs: Evaluated as low risk
  • src/mono/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.Mono.cs: Evaluated as low risk
  • src/libraries/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.cs: Evaluated as low risk
  • src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs: Evaluated as low risk
  • src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.NativeAot.cs: Evaluated as low risk
@teo-tsirpanis teo-tsirpanis force-pushed the typename-namespace-unescape branch from e1a9730 to d08be6b Compare January 20, 2025 22:19
@teo-tsirpanis teo-tsirpanis force-pushed the typename-namespace-unescape branch from d08be6b to 1e3f969 Compare January 20, 2025 22:30
teo-tsirpanis and others added 4 commits February 6, 2025 03:05
@teo-tsirpanis
Ignore ambiguous match exceptions in non-extensible Type.GetType ov…
cb2bd2d 
…erloads.

This matches the behavior of CoreCLR and Native AOT.
@teo-tsirpanis
Set TypeLoadException.TypeName in Mono as well.
02c2efe 
The property became init-only (internally) in order to avoid conflicts with runtime-specific constructor overloads.
@teo-tsirpanis @adamsitnik
Update src/libraries/System.Reflection.Metadata/tests/Metadata/TypeNa…
ed19a97 
…meTests.cs

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
@teo-tsirpanis
Merge branch 'main' into typename-namespace-unescape
0325ce1
@teo-tsirpanis teo-tsirpanis force-pushed the typename-namespace-unescape branch from d49ef0c to 0325ce1 Compare February 6, 2025 01:07
This was referenced Feb 6, 2025
Closed
Open
Open
@build-analysis build-analysis bot mentioned this pull request Feb 6, 2025
Open
3 tasks
@adamsitnik adamsitnik mentioned this pull request Feb 6, 2025
Closed
@teo-tsirpanis
Use alternative strategy to provide a (message, typeName) overload …
b924a6f 
…in Mono, while still avoiding changes to the Mono runtime code.
@teo-tsirpanis teo-tsirpanis force-pushed the typename-namespace-unescape branch from 87cf79d to b924a6f Compare February 8, 2025 16:06
This was referenced Feb 9, 2025
Open
Open
This was referenced Feb 10, 2025
Open
Open
@jkotas jkotas merged commit 617f9ee into dotnet:main Feb 10, 2025
138 of 143 checks passed
@teo-tsirpanis teo-tsirpanis deleted the typename-namespace-unescape branch February 10, 2025 22:20
@jkotas jkotas mentioned this pull request Feb 10, 2025
Open
@jkotas
Copy link
Member

jkotas commented Feb 10, 2025

I have opened #112376 as a follow up to use these APIs in more places.

@teo-tsirpanis teo-tsirpanis added this to the 10.0.0 milestone Feb 11, 2025
grendello added a commit to grendello/runtime that referenced this pull request Feb 11, 2025
@grendello
Merge branch 'main' into dev/grendel/android-clr-host-local
35c42cf 
* main:
  Code clean up in AP for NonNull* (dotnet#112027)
  JIT: Invalidate LSRA's DFS tree if we aren't running new layout phase (dotnet#112364)
  Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250204.2 (dotnet#112339)
  Add doc on OS onboarding (dotnet#112026)
  Add `TypeName` APIs to simplify metadata lookup. (dotnet#111598)
  Internal monitor impl not using coop mutex causing deadlocks on Android. (dotnet#112358)
  Do not run NAOT arm64 OSX testing on all PRs (dotnet#112342)
  Special-case empty enumerables in AsyncEnumerable (dotnet#112321)
  Have mono handle ConvertToIntegerNative for Double and Single (dotnet#112206)
  Update dependencies from https://github.com/dotnet/arcade build 20250206.4 (dotnet#112338)
  System.Configuration.ConfigurationManager.Tests: use Assembly.Location to determine ThisApplicationPath. (dotnet#112231)
  Force write of local file header when "version needed to extract" changes (dotnet#112032)
  JIT: Don't reorder handler blocks (dotnet#112292)
  [RISC-V] Synthesize some floating constants inline (dotnet#111529)
  Enable `SA1000`: Spacing around keywords (dotnet#112302)
  Fix relocs for linux-riscv64 AOT (dotnet#112331)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkotas jkotas jkotas approved these changes

Copilot code review Copilot Copilot left review comments

@adamsitnik adamsitnik adamsitnik approved these changes

@steveisok steveisok Awaiting requested review from steveisok steveisok is a code owner

@vitek-karas vitek-karas Awaiting requested review from vitek-karas vitek-karas is a code owner

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees

@adamsitnik adamsitnik

Labels
area-System.Reflection.Metadata community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation
Projects
None yet
Milestone
10.0.0
Development

Successfully merging this pull request may close these issues.

[API Proposal]: Simplify lookup of parsed TypeName in metadata
3 participants
@teo-tsirpanis @jkotas @adamsitnik