Avoid dereferencing null CheckConstraintsArgs.CurrentCompilation #78729
Conversation
| var testStruct = comp.GetTypeByMetadataName("TestStruct"); | ||
| var extensionMethodSymbol = comp.GetMember<MethodSymbol>("TestClass.TestExtensionMethod"); | ||
|
|
||
| AssertEx.Equal("void TestStruct.TestExtensionMethod<TestStruct>()", extensionMethodSymbol.ReduceExtensionMethod(testStruct, null).ToTestDisplayString()); |
There was a problem hiding this comment.
The comment on MethodSymbol.ReduceExtensionMethod may need to be updated. It says:
/// <summary>
/// If this is an extension method that can be applied to a receiver of the given type,
/// returns a reduced extension method symbol thus formed. Otherwise, returns null.
/// </summary>
/// <param name="compilation">The compilation in which constraints should be checked.
/// Should not be null, but if it is null we treat constraints as we would in the latest
/// language version.</param>
public MethodSymbol ReduceExtensionMethod(TypeSymbol receiverType, CSharpCompilation compilation, out bool wasFullyInferred)
But in fact, we're going to skip some constraint checks when the compilation is null.
That means we'll return symbols that violate some constraints. That may be worth adding a test to observe. #Resolved
There was a problem hiding this comment.
But in fact, we're going to skip some constraint checks when the compilation is null.
What constraint checks are getting skipped?
There was a problem hiding this comment.
We're skipping a check in CheckConstraintsSingleType
There was a problem hiding this comment.
We're skipping a check in
CheckConstraintsSingleType
It doesn't look like Binder.CheckManagedAddr actually checks any constraints. This helper used in many other settings, I guess it was convenient to include it here as well, as we do with some other checks that aren't primarily related to constraint checking. Besides, it doesn't look like CheckConstraintsSingleType is reachable from ReduceExtensionMethod
|
@dotnet/roslyn-compiler For another review |
2 similar comments
|
@dotnet/roslyn-compiler For another review |
|
@dotnet/roslyn-compiler For another review |
Fixes #78430.