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

Special case MakeGenericMethod/Type in RUC analyzer #2209

Merged
merged 5 commits into from
Sep 8, 2021

Conversation

mateoatr
Copy link
Contributor

Now that MakeGenericMethod and MakeGenericType methods were annotated in the runtime, we need to special case these in the analyzer to emit the right warning (IL2060 and IL2055, respectively).

{
if (member is IMethodSymbol method && ImmutableArrayOperations.Contains (specialIncompatibleMembers, member, SymbolEqualityComparer.Default)) {
if (method.Name == "MakeGenericType") {
operationContext.ReportDiagnostic (Diagnostic.Create (s_makeGenericTypeRule, operationContext.Operation.Syntax.GetLocation (), method.GetDisplayName ()));
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should simply not warn here.
The guiding principal for the analyzer should be:
"If the analyzer produces a warning -> linker will also produce that warning"

In this case this will now hold - linker has more complex handling of MakeGenericType/Method which will avoid warning in many cases. The analyzer would warn every time. I would much rather we special case this and not warn for now, until we can build enough smarts in the analyzer to match the linker behavior.

/cc @sbomer - we discussed the general behavior, so for context.

@mateoatr mateoatr force-pushed the makeGenericWarnings branch from 5903c6e to 3dc2dca Compare September 3, 2021 21:50
{
if (member is IMethodSymbol method && ImmutableArrayOperations.Contains (specialIncompatibleMembers, member, SymbolEqualityComparer.Default) &&
(method.Name == "MakeGenericMethod" || method.Name == "MakeGenericType")) {
return true;
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add a comment why we're doing this?

@mateoatr mateoatr merged commit 8b7695a into dotnet:main Sep 8, 2021
agocke pushed a commit to dotnet/runtime that referenced this pull request Nov 16, 2022
…2209)

* Add MakeGenericMethod and MakeGenericType to the special incompatible members of the RUC analyzer.

* Don't produce diagnostics for MakeGenericMethod/MakeGenericType

* Add comment

* Lint

Commit migrated from dotnet/linker@8b7695a
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