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 empty/null handling for Type.BaseType intrinsic #2694

Merged
merged 1 commit into from
Mar 18, 2022

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Mar 18, 2022

@vitek-karas this should fix some of the issues you are seeing with dataflow in foreach loops involving Type.BaseType.

@vitek-karas shared the following testcase which was warning:

// Can only work with All annotation as NonPublicProperties doesn't propagate to base types
static void EnumeratePrivatePropertiesOnBaseTypesWithForeach ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)] Type type)
{
	const BindingFlags DeclaredOnlyLookup = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly;
	Type? t = type;
	while (t != null) {
		foreach (var p in t.GetProperties (DeclaredOnlyLookup)) {
			// Do nothing
		}
		t = t.BaseType;
	}
}

This was producing a warning because during the first pass of the analysis over t.BaseType, the analyzer hadn't yet figured out that t had annotation All. (The reason for this has to do with exception handling - the statement is reachable only through a finally block which we hadn't visited yet). BaseType gave back None, which produced a warning on the call to GetProperties.

What this indicates to me is that dotnet/runtime#106886 is important not just to be precise about the warning behavior when there are empty inputs, but also for the correctness of the solver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants