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.

Sign up for GitHub

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

Jump to bottom

Update BoundCall method based on receiver nullability #31000

Merged
merged 4 commits into from
Nov 9, 2018
Merged

Update BoundCall method based on receiver nullability #31000

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5fd9ae6
Update BoundCall method based on receiver nullability
cston Nov 6, 2018
ad73072
Additional tests
cston Nov 7, 2018
340a850
Misc.
cston Nov 7, 2018
9da8b69
Fix tests
cston Nov 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1953,11 +1953,12 @@ public override BoundNode VisitCall(BoundCall node)
var receiverOpt = node.ReceiverOpt;
if (receiverOpt != null && method.MethodKind != MethodKind.Constructor)
{
VisitRvalue(receiverOpt);
var receiverType = VisitRvalueWithResult(receiverOpt);
// https://github.com/dotnet/roslyn/issues/30598: Mark receiver as not null
// after arguments have been visited, and only if the receiver has not changed.
CheckPossibleNullReceiver(receiverOpt);
// Update method based on inferred receiver type: see https://github.com/dotnet/roslyn/issues/29605.
// Update method based on inferred receiver type.
method = (MethodSymbol)AsMemberOfResultType(receiverType, method);
}

// https://github.com/dotnet/roslyn/issues/29605 Can we handle some error cases?
Expand Down Expand Up @@ -2797,9 +2798,9 @@ private TypeSymbolWithAnnotations GetAdjustedResult(TypeSymbolWithAnnotations ty
return type;
}

private Symbol AsMemberOfResultType(Symbol symbol)
private static Symbol AsMemberOfResultType(TypeSymbolWithAnnotations resultType, Symbol symbol)
{
var containingType = _resultType.TypeSymbol as NamedTypeSymbol;
var containingType = resultType.TypeSymbol as NamedTypeSymbol;
if ((object)containingType == null || containingType.IsErrorType())
{
return symbol;
Expand All @@ -2813,6 +2814,9 @@ private static Symbol AsMemberOfType(NamedTypeSymbol containingType, Symbol symb
{
return null;
}
#if DEBUG
bool wasInterface = containingType.IsInterface;
#endif
if (symbol.Kind == SymbolKind.Method)
{
if (((MethodSymbol)symbol).MethodKind == MethodKind.LocalFunction)
Expand All @@ -2831,16 +2835,23 @@ private static Symbol AsMemberOfType(NamedTypeSymbol containingType, Symbol symb
{
return AsMemberOfTupleType((TupleTypeSymbol)containingType, symbol);
}
return symbolDef.SymbolAsMember(containingType);
var result = symbolDef.SymbolAsMember(containingType);
if (result is MethodSymbol resultMethod && resultMethod.IsGenericMethod)
{
return resultMethod.Construct(((MethodSymbol)symbol).TypeArguments);
}
return result;
}
containingType = containingType.BaseTypeNoUseSiteDiagnostics;
if ((object)containingType == null)
{
break;
}
}
// https://github.com/dotnet/roslyn/issues/29967 Handle other cases such as interfaces.
Debug.Assert(symbolDefContainer.IsInterface);
#if DEBUG
// https://github.com/dotnet/roslyn/issues/29967 Handle interfaces.
Debug.Assert(wasInterface);
#endif
return symbol;
}

Expand Down Expand Up @@ -3725,7 +3736,7 @@ private void VisitMemberAccess(BoundExpression receiverOpt, Symbol member, bool

if (!member.IsStatic)
{
member = AsMemberOfResultType(member);
member = AsMemberOfResultType(_resultType, member);
// https://github.com/dotnet/roslyn/issues/30598: For l-values, mark receiver as not null
// after RHS has been visited, and only if the receiver has not changed.
CheckPossibleNullReceiver(receiverOpt);
Expand Down
107 changes: 107 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51406,5 +51406,112 @@ static void Main()
var comp = CreateCompilation(source2, new[] { ref1.WithEmbedInteropTypes(true), CSharpRef }, options: WithNonNullTypesTrue(TestOptions.ReleaseExe));
CompileAndVerify(comp, expectedOutput: "4");
}

[WorkItem(29605, "https://github.com/dotnet/roslyn/issues/29605")]
[Fact]
public void InferMethodReceiverType_01()
Copy link
Member

@jcouv jcouv Nov 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Is this issue specific to methods? Would it make sense to also test properties or fields?
Oh, I see that you updated VisitMemberAccess too, which covers fields, properties and events. It may still be good to add a test for those (if not covered already). #Closed

Copy link
Member Author

@cston cston Nov 7, 2018 •
edited by jcouv
Loading

Choose a reason for hiding this comment

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

There are a number of existing tests that rely on the update in VisitMemberAccess, but I've added explicit tests here. #Closed

{
var source =
@"class C<T>
{
internal void F(T t) { }
}
class Program
{
internal static C<T> Create<T>(T t) => throw null;
static void M(object x, object? y, string? z)
{
var c = Create(x);
c.F(x);
c.F(y); // 1
c.F(z); // 2
c.F(null); // 3
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (12,13): warning CS8604: Possible null reference argument for parameter 't' in 'void C<object>.F(object t)'.
// c.F(y); // 1
Diagnostic(ErrorCode.WRN_NullReferenceArgument, "y").WithArguments("t", "void C<object>.F(object t)").WithLocation(12, 13),
// (13,13): warning CS8604: Possible null reference argument for parameter 't' in 'void C<object>.F(object t)'.
// c.F(z); // 2
Diagnostic(ErrorCode.WRN_NullReferenceArgument, "z").WithArguments("t", "void C<object>.F(object t)").WithLocation(13, 13),
// (14,13): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter.
// c.F(null); // 3
Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(14, 13));
}

[WorkItem(29605, "https://github.com/dotnet/roslyn/issues/29605")]
[Fact]
public void InferMethodReceiverType_02()
{
var source =
@"class A<T> { }
class B<T>
{
internal void F<U>(U u) where U : A<T> { }
}
class Program
{
internal static B<T> Create<T>(T t) => throw null;
static void M1(object x, A<object> y, A<object?> z)
{
var b1 = Create(x);
b1.F(y);
b1.F(z); // 1
}
static void M2(object? x, A<object> y, A<object?> z)
{
var b2 = Create(x);
b2.F(y); // 2
b2.F(z);
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (13,9): warning CS8631: The type 'A<object?>' cannot be used as type parameter 'U' in the generic type or method 'B<object>.F<U>(U)'. Nullability of type argument 'A<object?>' doesn't match constraint type 'A<object>'.
// b1.F(z); // 1
Diagnostic(ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint, "b1.F").WithArguments("B<object>.F<U>(U)", "A<object>", "U", "A<object?>").WithLocation(13, 9),
// (18,9): warning CS8631: The type 'A<object>' cannot be used as type parameter 'U' in the generic type or method 'B<object?>.F<U>(U)'. Nullability of type argument 'A<object>' doesn't match constraint type 'A<object?>'.
// b2.F(y); // 2
Diagnostic(ErrorCode.WRN_NullabilityMismatchInTypeParameterConstraint, "b2.F").WithArguments("B<object?>.F<U>(U)", "A<object?>", "U", "A<object>").WithLocation(18, 9));
}

[WorkItem(29605, "https://github.com/dotnet/roslyn/issues/29605")]
[Fact]
public void InferMethodReceiverType_03()
{
var source =
@"class C<T>
{
internal static T F() => throw null;
}
class Program
{
internal static C<T> Create<T>(T t) => throw null;
static void M(object x)
{
var c1 = Create(x);
c1.F().ToString();
x = null; // 1
var c2 = Create(x);
c2.F().ToString(); // 2
}
}";
var comp = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics(
// (11,9): error CS0176: Member 'C<object>.F()' cannot be accessed with an instance reference; qualify it with a type name instead
// c1.F().ToString();
Diagnostic(ErrorCode.ERR_ObjectProhibited, "c1.F").WithArguments("C<object>.F()").WithLocation(11, 9),
// (12,13): warning CS8600: Converting null literal or possible null value to non-nullable type.
// x = null; // 1
Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(12, 13),
// (14,9): error CS0176: Member 'C<object>.F()' cannot be accessed with an instance reference; qualify it with a type name instead
// c2.F().ToString(); // 2
Diagnostic(ErrorCode.ERR_ObjectProhibited, "c2.F").WithArguments("C<object>.F()").WithLocation(14, 9),
// (14,9): warning CS8602: Possible dereference of a null reference.
// c2.F().ToString(); // 2
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c2.F()").WithLocation(14, 9));
}
}
}