Skip to content

Trim ILTrim parameter names selectively - #134087

Draft
jtschuster wants to merge 1 commit into
dev/jtschuster-copilot/ILTrim/descriptor-resource-parsingfrom
dev/jtschuster-copilot/ILTrim/parameter-name-trimming
Draft

jtschuster wants to merge 1 commit into
dev/jtschuster-copilot/ILTrim/descriptor-resource-parsingfrom
dev/jtschuster-copilot/ILTrim/parameter-name-trimming

Conversation

@jtschuster

@jtschuster jtschuster commented Sep 16, 2026

Copy link
Copy Markdown
Member

The draft PRs in this stack I haven't reviewed myself and don't expect anyone else to until I mark them ready for review (unless you want to). If the tags are causing too much noise for anyone I can move them to my fork until their ready.

Layer 9 of 12 in the ILTrim expected-failure reduction stack.

Remove parameter names when they are not reflection-visible while retaining names required by reflected methods and descriptors.

Stack-top validation

  • Release ILTrim: 928 total, 424 passed, 504 skipped, 0 failed, 0 errors.
  • ./build.sh tools+tools.ilasm+tools.illinktests+tools.cdactests -test passed with 0 warnings and 0 errors.
  • ILCompiler.Compiler Release build passed with 0 warnings and 0 errors.

Stack created with GitHub Stacks CLIGive Feedback 💬

Note

This PR description was generated by GitHub Copilot.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@jtschuster jtschuster changed the title Integrate ILTrim parameter name trimming Trim ILTrim parameter names selectively Sep 16, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/crossgen-contrib
See info in area-owners.md if you want to be subscribed.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib
See info in area-owners.md if you want to be subscribed.

@jkotas jkotas added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed area-ILTools-coreclr labels Sep 17, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/illink
See info in area-owners.md if you want to be subscribed.

Comment on lines 333 to +335
// TODO: this should be a separate node with more logic
var definition = (EcmaMethod)method.GetTypicalMethodDefinition();
_reflectedMethods.TryAdd(new HandleKey<MethodDefinitionHandle>(definition.Module, definition.Handle), 0);

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.

I think we should just have a ReflectedMethodNode, same as ILC has. We'll probably need to condition things on such node later (for example, if an interface method is reflected, all the implementations of the interface method should also be considered reflected because one can just create a delegate to an interface method and then query its MethodInfo).

ParameterNode can then just query .IsMarked on the owning method ReflectedMethodNode.

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.

Mmm, looks like we lack API (equivalent of #60380) to get from parameter handle to method.

We can do something slow to workaround for now and file the issue to get an API.

@jtschuster
jtschuster removed this pull request from stack #134091 September 17, 2026 17:27
@jtschuster
jtschuster force-pushed the dev/jtschuster-copilot/ILTrim/parameter-name-trimming branch from 939eedc to 79fd706 Compare September 17, 2026 17:28
@jtschuster
jtschuster added this pull request to stack #134146 September 17, 2026 17:28
Squash accepted iteration-8 slot 3 from b41340e1cd422b268fcac0ecfa56430925516a08.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e06ebffa-ad53-49ba-b7aa-2efb4ee16d92
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Tools-ILLink .NET linker development as well as trimming analyzers

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants