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 an interesting test case for compiler generated code marking #2928

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

vitek-karas
Copy link
Member

See the test for description of what it does. It's basically the linker repro for the case hit in dotnet/runtime#73027 (comment)

See the test for description of what it does. It's basically the linker repro for the case hit in dotnet/runtime#73027 (comment)
Copy link
Member

@sbomer sbomer left a comment

Choose a reason for hiding this comment

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

Interesting, thanks!

Is the warning behavior what you would expect? In theory the descriptor marking should produce the same kinds of warnings as reflection access to compiler-generated code, but that might be too noisy.

So the fix would be to make the processing of both cases consistent, but it wouldn't change the warnings or the output, right?

@vitek-karas
Copy link
Member Author

Right - the behavior currently seems OK to me - we don't produce warning in either case. Basically even though the entire assembly is preserved, we treat the compiler generated code as "trimmable" anyway (we would still keep it, but we won't produce diagnostics from it). I think that's OK, less noise and I don't see any holes (we don't see any reference to it anywhere).

The internal behavior is obviously weird - it would be great if we could make it to be consistent regardless of ordering. But I could not find any observable differences yet.

@vitek-karas vitek-karas merged commit 463997e into dotnet:main Jul 29, 2022
@vitek-karas vitek-karas deleted the TestForCGState branch July 29, 2022 20:18
agocke pushed a commit to dotnet/runtime that referenced this pull request Nov 16, 2022
…net/linker#2928)

See the test for description of what it does. It's basically the linker repro for the case hit in #73027 (comment)

Commit migrated from dotnet/linker@463997e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants