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

Clarify error message when this is returned by reference #15706

Merged
merged 2 commits into from
May 18, 2017
Merged

Clarify error message when this is returned by reference #15706

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a56b835
Clarify error message when this is returned by reference
alrz Dec 6, 2016
baf771d
Merge remote-tracking branch 'origin/master' into issue-15704
alrz May 5, 2017
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
10 changes: 7 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1147,8 +1147,6 @@ static private ErrorCode GetThisLvalueError(BindValueKind kind)
return ErrorCode.ERR_AddrOnReadOnlyLocal;
case BindValueKind.IncrementDecrement:
return ErrorCode.ERR_IncrementLvalueExpected;
case BindValueKind.RefReturn:
return ErrorCode.ERR_RefReturnStructThis;
}
}

Expand Down Expand Up @@ -1572,9 +1570,15 @@ private bool CheckIsVariable(SyntaxNode node, BoundExpression expr, BindValueKin
var thisref = expr as BoundThisReference;
if (thisref != null)
{
if (kind == BindValueKind.RefReturn)
{
Error(diagnostics, thisref.Type.IsValueType ? ErrorCode.ERR_RefReturnStructThis : ErrorCode.ERR_RefReadonlyLocal, node, ThisParameterSymbol.SymbolName);
return false;
}

// We will already have given an error for "this" used outside of a constructor,
// instance method, or instance accessor. Assume that "this" is a variable if it is in a struct.
if (!thisref.Type.IsValueType || kind == BindValueKind.RefReturn)
if (!thisref.Type.IsValueType)
{
// CONSIDER: the Dev10 name has angle brackets (i.e. "<this>")
Error(diagnostics, GetThisLvalueError(kind), node, ThisParameterSymbol.SymbolName);
Expand Down
65 changes: 37 additions & 28 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RefLocalsAndReturnsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,37 +595,46 @@ static ref char Test1()
}
}


public class C
{
public ref C M()
{
return ref this;
}
}
}";
var comp = CreateCompilationRef(text);
comp.VerifyDiagnostics(
// (10,24): error CS8170: Struct members cannot return 'this' or other instance members by reference
// return ref this;
Diagnostic(ErrorCode.ERR_RefReturnStructThis, "this").WithArguments("this").WithLocation(10, 24),
// (15,24): error CS8170: Struct members cannot return 'this' or other instance members by reference
// return ref x;
Diagnostic(ErrorCode.ERR_RefReturnStructThis, "x").WithArguments("this").WithLocation(15, 24),
// (20,24): error CS8170: Struct members cannot return 'this' or other instance members by reference
// return ref this.x;
Diagnostic(ErrorCode.ERR_RefReturnStructThis, "this.x").WithArguments("this").WithLocation(20, 24),
// (36,32): error CS8168: Cannot return local 'M1' by reference because it is not a ref local
// return ref Foo(ref M1);
Diagnostic(ErrorCode.ERR_RefReturnLocal, "M1").WithArguments("M1").WithLocation(36, 32),
// (36,24): error CS8164: Cannot return by reference a result of 'Test.Foo<char>(ref char)' because the argument passed to parameter 'arg' cannot be returned by reference
// return ref Foo(ref M1);
Diagnostic(ErrorCode.ERR_RefReturnCall, "Foo(ref M1)").WithArguments("Test.Foo<char>(ref char)", "arg").WithLocation(36, 24),
// (41,32): error CS8169: Cannot return a member of local 'M2' by reference because it is not a ref local
// return ref Foo(ref M2.x);
Diagnostic(ErrorCode.ERR_RefReturnLocal2, "M2").WithArguments("M2").WithLocation(41, 32),
// (41,24): error CS8164: Cannot return by reference a result of 'Test.Foo<char>(ref char)' because the argument passed to parameter 'arg' cannot be returned by reference
// return ref Foo(ref M2.x);
Diagnostic(ErrorCode.ERR_RefReturnCall, "Foo(ref M2.x)").WithArguments("Test.Foo<char>(ref char)", "arg").WithLocation(41, 24),
// (46,32): error CS8168: Cannot return local 'M2' by reference because it is not a ref local
// return ref Foo(ref M2).x;
Diagnostic(ErrorCode.ERR_RefReturnLocal, "M2").WithArguments("M2").WithLocation(46, 32),
// (46,24): error CS8165: Cannot return by reference a member of result of 'Test.Foo<Test.S1>(ref Test.S1)' because the argument passed to parameter 'arg' cannot be returned by reference
// return ref Foo(ref M2).x;
Diagnostic(ErrorCode.ERR_RefReturnCall2, "Foo(ref M2)").WithArguments("Test.Foo<Test.S1>(ref Test.S1)", "arg").WithLocation(46, 24));
// (10,24): error CS8170: Struct members cannot return 'this' or other instance members by reference
// return ref this;
Diagnostic(ErrorCode.ERR_RefReturnStructThis, "this").WithArguments("this").WithLocation(10, 24),
// (15,24): error CS8170: Struct members cannot return 'this' or other instance members by reference
// return ref x;
Diagnostic(ErrorCode.ERR_RefReturnStructThis, "x").WithArguments("this").WithLocation(15, 24),
// (20,24): error CS8170: Struct members cannot return 'this' or other instance members by reference
// return ref this.x;
Diagnostic(ErrorCode.ERR_RefReturnStructThis, "this.x").WithArguments("this").WithLocation(20, 24),
// (36,32): error CS8168: Cannot return local 'M1' by reference because it is not a ref local
// return ref Foo(ref M1);
Diagnostic(ErrorCode.ERR_RefReturnLocal, "M1").WithArguments("M1").WithLocation(36, 32),
// (36,24): error CS8164: Cannot return by reference a result of 'Test.Foo<char>(ref char)' because the argument passed to parameter 'arg' cannot be returned by reference
// return ref Foo(ref M1);
Diagnostic(ErrorCode.ERR_RefReturnCall, "Foo(ref M1)").WithArguments("Test.Foo<char>(ref char)", "arg").WithLocation(36, 24),
// (41,32): error CS8169: Cannot return a member of local 'M2' by reference because it is not a ref local
// return ref Foo(ref M2.x);
Diagnostic(ErrorCode.ERR_RefReturnLocal2, "M2").WithArguments("M2").WithLocation(41, 32),
// (41,24): error CS8164: Cannot return by reference a result of 'Test.Foo<char>(ref char)' because the argument passed to parameter 'arg' cannot be returned by reference
// return ref Foo(ref M2.x);
Diagnostic(ErrorCode.ERR_RefReturnCall, "Foo(ref M2.x)").WithArguments("Test.Foo<char>(ref char)", "arg").WithLocation(41, 24),
// (46,32): error CS8168: Cannot return local 'M2' by reference because it is not a ref local
// return ref Foo(ref M2).x;
Diagnostic(ErrorCode.ERR_RefReturnLocal, "M2").WithArguments("M2").WithLocation(46, 32),
// (46,24): error CS8165: Cannot return by reference a member of result of 'Test.Foo<Test.S1>(ref Test.S1)' because the argument passed to parameter 'arg' cannot be returned by reference
// return ref Foo(ref M2).x;
Diagnostic(ErrorCode.ERR_RefReturnCall2, "Foo(ref M2)").WithArguments("Test.Foo<Test.S1>(ref Test.S1)", "arg").WithLocation(46, 24),
// (58,24): error CS1605: Cannot use 'this' as a ref or out value because it is read-only
// return ref this;
Diagnostic(ErrorCode.ERR_RefReadonlyLocal, "this").WithArguments("this").WithLocation(58, 24));
Copy link
Member Author

@alrz alrz Dec 14, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This case did not have a test, I added it to RefReadonlyCall. Should I separate it out?

Copy link
Member

Choose a reason for hiding this comment

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

Its fine... are you saying the diagnostics for this case were different before?

Copy link
Member Author

@alrz alrz Dec 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, it was CS8170 regardless of the containing type being class or struct.

}

[Fact]
Expand Down