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

Fix CA2021 in MethodBodyScanner #81200

Merged
merged 1 commit into from
Jan 26, 2023
Merged

Conversation

MichalStrehovsky
Copy link
Member

#80916 (comment) for context.

Cc @dotnet/ilc-contrib

@ghost
Copy link

ghost commented Jan 26, 2023

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

Issue Details

#80916 (comment) for context.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@@ -319,7 +319,7 @@ public virtual void InterproceduralScan(MethodIL startingMethodBody)
// are the same set of methods that we discovered and scanned above.
if (_annotations.CompilerGeneratedState.TryGetCompilerGeneratedCalleesForUserMethod(startingMethod, out List<TypeSystemEntity>? compilerGeneratedCallees))
{
var calleeMethods = compilerGeneratedCallees.OfType<MethodDefinition>();
var calleeMethods = compilerGeneratedCallees.OfType<MethodDesc>();
Copy link
Member

Choose a reason for hiding this comment

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

Since OfType evaluation is lazy, this line is a nop (other than a null check on compilerGeneratedCallees). Presumably you could comment it out to go along with the subsequent commented out lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would work too. I went with this because this file is a line-by-line port from IL Linker where it looks like this:

var calleeMethods = compilerGeneratedCallees.OfType<MethodDefinition> ();
// https://github.com/dotnet/linker/issues/2845
// Disabled asserts due to a bug
// Debug.Assert (interproceduralState.Count == 1 + calleeMethods.Count ());
// foreach (var method in calleeMethods)
// Debug.Assert (interproceduralState.Any (kvp => kvp.Key.Method == method));

(It's identical modulo the odd illinker rule to put a space between method name and parameter list. You can see where the MethodDefinition came from. It happened to compile here, but it's still the wrong type.)

@MichalStrehovsky MichalStrehovsky merged commit 301cb81 into main Jan 26, 2023
@MichalStrehovsky MichalStrehovsky deleted the MichalStrehovsky-patch-1 branch January 26, 2023 09:30
@ghost ghost locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants