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.

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

Add ILLink annotations to Microsoft.CSharp #50239

Merged
merged 5 commits into from
Apr 12, 2021
Merged

Conversation

krwq
Copy link
Member

@krwq krwq commented Mar 25, 2021

This adds ILLink annotations to Microsoft.CSharp. Majority is RequiresUnreferencedCode.

@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, to 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.

@ghost
Copy link

ghost commented Mar 25, 2021

Tagging subscribers to this area: @cston
See info in area-owners.md if you want to be subscribed.

Issue Details

This adds ILLink annotations to Microsoft.CSharp. Majority is RequiresUnreferencedCode.

Author: krwq
Assignees: -
Labels:

area-Microsoft.CSharp, new-api-needs-documentation

Milestone: -

@MichalStrehovsky
Copy link
Member

Do we want a better message for this than "Types and members might be removed"?

How does the experience look like if I have:

dynamic foo = "Hello";
dynamic bar = "World";
Console.WriteLine(foo + bar);

It would be nice if there was indication that this is caused by the dynamic keyword. Developers who use C# dynamic typically don't know how it's implemented. I've seen code written in ways that very much looked like someone didn't understand the difference between var and dynamic.

@eerhardt eerhardt requested a review from joperezr March 26, 2021 14:06
@eerhardt eerhardt added linkable-framework Issues associated with delivering a linker friendly framework and removed new-api-needs-documentation labels Mar 26, 2021
@ghost
Copy link

ghost commented Mar 26, 2021

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @tannergooding, @sbomer
See info in area-owners.md if you want to be subscribed.

Issue Details

This adds ILLink annotations to Microsoft.CSharp. Majority is RequiresUnreferencedCode.

Author: krwq
Assignees: -
Labels:

area-Microsoft.CSharp, linkable-framework

Milestone: -

@eerhardt
Copy link
Member

Looks like your new trimming test is failing on mono -

Type `System.Runtime.InteropServices.COMException` does not have constructor
The command "/__w/1/s/artifacts/bin/trimmingTests/projects/MarshalGetExceptionForHRTests.TrimmingTests/MarshalGetExceptionForHRTests/browser-wasm/bin/Release/net6.0/browser-wasm/AppBundle/run-v8.sh" exited with code 255.

@krwq
Copy link
Member Author

krwq commented Mar 29, 2021

@MichalStrehovsky I've changed message to Using dynamic types might cause types or members to be removed by trimmer., let me know if you have better idea for wording

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM. I had just a couple minor comments, once those are addressed I think we should merge this. Thanks for the good work here, @krwq!

@dotnet dotnet deleted a comment from azure-pipelines bot Apr 12, 2021
@dotnet dotnet deleted a comment from azure-pipelines bot Apr 12, 2021
@krwq krwq closed this Apr 12, 2021
@krwq krwq reopened this Apr 12, 2021
@krwq krwq closed this Apr 12, 2021
@krwq krwq reopened this Apr 12, 2021
@dotnet dotnet deleted a comment from azure-pipelines bot Apr 12, 2021
@dotnet dotnet deleted a comment from azure-pipelines bot Apr 12, 2021
@dotnet dotnet deleted a comment from azure-pipelines bot Apr 12, 2021
@dotnet dotnet deleted a comment from azure-pipelines bot Apr 12, 2021
@krwq krwq merged commit d2cf673 into dotnet:main Apr 12, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 12, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Microsoft.CSharp linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants