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

readonly ref local functions, error tolerance in parsing, more tests. #17223

Merged
merged 2 commits into from
Feb 21, 2017
Merged

readonly ref local functions, error tolerance in parsing, more tests. #17223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9474f96
More parsing tests for `readonly ref` returns
VSadov Feb 17, 2017
85e40b7
CR feedback
VSadov Feb 21, 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
14 changes: 11 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 @@ -786,10 +786,18 @@ private TypeSymbol BindVariableType(CSharpSyntaxNode declarationNode, Diagnostic
// might own nested scope.
bool hasErrors = localSymbol.ScopeBinder.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics);

var containingMethod = this.ContainingMemberOrLambda as MethodSymbol;
if (containingMethod != null && containingMethod.IsAsync && localSymbol.RefKind != RefKind.None)
if (localSymbol.RefKind == RefKind.In)
{
Error(diagnostics, ErrorCode.ERR_BadAsyncLocalType, declarator);
var refKeyword = typeSyntax.GetFirstToken();
diagnostics.Add(ErrorCode.ERR_UnexpectedToken, refKeyword.GetLocation(), refKeyword.ToString());
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

diagnostics.Add(ErrorCode.ERR_UnexpectedToken, refKeyword.GetLocation(), refKeyword.ToString()); [](start = 16, length = 96)

Could you please point to the test that goes through this code path? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

ReadonlyRefReturn_UnexpectedBindTime

We allow readonly ref locals at parse time for error recovery purposes, but will fail at bind time.

This is the same strategy as used for regular ref. We parse it as RefType in more places than where it is semantically allowed, but fail to bind with this error.

Also there is a possibility we would allow readonly ref locals. That is another reason for having the check at bind time.

In reply to: 102299405 [](ancestors = 102299405)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am probably missing something, but I don't see this error reported for a local declaration in that test.

In reply to: 102323203 [](ancestors = 102323203,102299405)

Copy link
Member Author

Choose a reason for hiding this comment

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

Because binding is NYI and we fallback on "Ref", which is valid.
I will add PROTOTYPE note

In reply to: 102336648 [](ancestors = 102336648,102323203,102299405)

}
else
{
var containingMethod = this.ContainingMemberOrLambda as MethodSymbol;
if (containingMethod != null && containingMethod.IsAsync && localSymbol.RefKind != RefKind.None)
{
Error(diagnostics, ErrorCode.ERR_BadAsyncLocalType, declarator);
}
}

EqualsValueClauseSyntax equalsClauseSyntax = declarator.Initializer;
Expand Down
16 changes: 14 additions & 2 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6258,8 +6258,14 @@ private ScanTypeFlags ScanNonArrayType(ParseTypeMode mode, out SyntaxToken lastT
{
ScanTypeFlags result;

// in a ref local or ref return, we treat "ref" as part of the type
if (this.CurrentToken.Kind == SyntaxKind.RefKeyword)
// in a ref local or ref return, we treat "ref" and "readonly ref" as part of the type
if (this.CurrentToken.Kind == SyntaxKind.ReadOnlyKeyword &&
this.PeekToken(1).Kind == SyntaxKind.RefKeyword)
{
this.EatToken();
this.EatToken();
}
else if (this.CurrentToken.Kind == SyntaxKind.RefKeyword)
{
this.EatToken();
}
Expand Down Expand Up @@ -8743,6 +8749,12 @@ private void ParseDeclarationModifiers(SyntaxListBuilder list)
SyntaxKind k;
while (IsDeclarationModifier(k = this.CurrentToken.ContextualKind) || IsAdditionalLocalFunctionModifier(k))
{
// "readonly ref" is not a modifier, in our syntax.
if (k == SyntaxKind.ReadOnlyKeyword && this.PeekToken(1).Kind == SyntaxKind.RefKeyword)
{
break;
}

SyntaxToken mod;
if (k == SyntaxKind.AsyncKeyword)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenReadonlyRefReturnTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
[CompilerTrait(CompilerFeature.ReadonlyReferences)]
public class ReadonlyRefReturnTests : CompilingTestBase
public class CodeGenReadonlyRefReturnTests : CompilingTestBase
{
[Fact]
public void RefReturnArrayAccess()
Expand All @@ -29,7 +29,7 @@ class Program
";

//PROTOTYPE(readonlyRefs): this should work for now because readonly is treated as regular ref
var comp = CompileAndVerify(text, parseOptions: TestOptions.Latest);
var comp = CompileAndVerify(text, parseOptions: TestOptions.Regular);

comp.VerifyIL("Program.M()", @"
{
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Symbol/Symbols/CompilationCreationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2717,7 +2717,7 @@ public void CompilationWithReferenceDirectives_Errors()
", options: TestOptions.Script),
SyntaxFactory.ParseSyntaxTree(@"
#r ""System.Core""
")
", TestOptions.Regular)
};

var compilation = CreateCompilationWithMscorlib45(
Expand Down Expand Up @@ -2800,7 +2800,7 @@ class C
{
void Foo() { Console.WriteLine(3); }
}
")
", TestOptions.Regular)
};

var compilation = CreateCompilationWithMscorlib45(
Expand Down
15 changes: 15 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/Symbols/Source/DelegateTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,5 +731,20 @@ public void RefReturningDelegate()
Assert.Equal(RefKind.Ref, d.DelegateInvokeMethod.RefKind);
Assert.Equal(RefKind.Ref, ((MethodSymbol)d.GetMembers("EndInvoke").Single()).RefKind);
}

[Fact]
public void ReadonlyRefReturningDelegate()
{
var source = @"delegate readonly ref int D();";

var comp = CreateCompilationWithMscorlib45(source);
comp.VerifyDiagnostics();

var global = comp.GlobalNamespace;
var d = global.GetMembers("D")[0] as NamedTypeSymbol;
//PROTOTYPE(readonlyRefeences): this will work as regular RefKind.Ref for now since binding is NYI
Assert.Equal(RefKind.Ref, d.DelegateInvokeMethod.RefKind);
Assert.Equal(RefKind.Ref, ((MethodSymbol)d.GetMembers("EndInvoke").Single()).RefKind);
}
}
}
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/Symbols/Source/ExpressionBodiedMethodTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,18 @@ class C
{
int field = 0;
public ref int M() => ref field;
}");
comp.VerifyDiagnostics();
}

[Fact]
public void ReadonlyRefReturningExpressionBodiedMethod()
{
var comp = CreateCompilationWithMscorlib45(@"
class C
{
int field = 0;
public readonly ref int M() => ref field;
}");
comp.VerifyDiagnostics();
}
Expand Down
23 changes: 23 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/Symbols/Source/ExpressionBodiedPropertyTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,5 +504,28 @@ class C
Assert.True(p.IsExpressionBodied);
Assert.Equal(RefKind.Ref, p.GetMethod.RefKind);
}

[Fact]
public void ReadonlyRefReturningExpressionBodiedProperty()
{
var comp = CreateCompilationWithMscorlib45(@"
class C
{
int field = 0;
public readonly ref int P => ref field;
}");
comp.VerifyDiagnostics();

var global = comp.GlobalNamespace;
var c = global.GetTypeMember("C");

var p = c.GetMember<SourcePropertySymbol>("P");
Assert.Null(p.SetMethod);
Assert.NotNull(p.GetMethod);
Assert.False(p.GetMethod.IsImplicitlyDeclared);
Assert.True(p.IsExpressionBodied);
//PROTOTYPE(readonlyRef): binding is currently NYI so it is "Ref" for now.
Assert.Equal(RefKind.Ref, p.GetMethod.RefKind);
}
}
}
70 changes: 68 additions & 2 deletions src/Compilers/CSharp/Test/Symbol/Symbols/Source/MethodTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2073,6 +2073,23 @@ static class C
);
}

[Fact]
public void ReadonlyRefReturningVoidMethod()
{
var source = @"
static class C
{
static readonly ref void M() { }
}
";

CreateCompilationWithMscorlib45(source).VerifyDiagnostics(
// (4,25): error CS1547: Keyword 'void' cannot be used in this context
// static readonly ref void M() { }
Diagnostic(ErrorCode.ERR_NoVoidHere, "void").WithLocation(4, 25)
);
}

[Fact]
public void RefReturningVoidMethodNested()
{
Expand All @@ -2086,8 +2103,7 @@ static void Main()
}
";

var parseOptions = TestOptions.Regular;
CreateCompilationWithMscorlib45(source, parseOptions: parseOptions).VerifyDiagnostics(
CreateCompilationWithMscorlib45(source).VerifyDiagnostics(
// (6,13): error CS1547: Keyword 'void' cannot be used in this context
// ref void M() { }
Diagnostic(ErrorCode.ERR_NoVoidHere, "void").WithLocation(6, 13),
Expand All @@ -2097,6 +2113,33 @@ static void Main()
);
}

[Fact]
public void ReadonlyRefReturningVoidMethodNested()
{
var source = @"
static class C
{
static void Main()
{
// valid
readonly ref int M1() {throw null;}

// not valid
readonly ref void M2() {M1(); throw null;}

M2();
}
}
";

var parseOptions = TestOptions.Regular;
CreateCompilationWithMscorlib45(source).VerifyDiagnostics(
// (10,22): error CS1547: Keyword 'void' cannot be used in this context
// readonly ref void M2() {M1(); throw null;}
Diagnostic(ErrorCode.ERR_NoVoidHere, "void").WithLocation(10, 22)
);
}

[Fact]
public void RefReturningAsyncMethod()
{
Expand All @@ -2119,5 +2162,28 @@ static class C
Diagnostic(ErrorCode.ERR_ReturnExpected, "M").WithArguments("C.M()").WithLocation(4, 26)
);
}

[Fact]
public void ReadonlyRefReturningAsyncMethod()
{
var source = @"
static class C
{
static async readonly ref int M() { }
}
";

CreateCompilationWithMscorlib45(source).VerifyDiagnostics(
// (4,18): error CS1073: Unexpected token 'readonly'
// static async readonly ref int M() { }
Diagnostic(ErrorCode.ERR_UnexpectedToken, "readonly").WithArguments("readonly").WithLocation(4, 18),
// (4,35): warning CS1998: This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
// static async readonly ref int M() { }
Diagnostic(ErrorCode.WRN_AsyncLacksAwaits, "M").WithLocation(4, 35),
// (4,35): error CS0161: 'C.M()': not all code paths return a value
// static async readonly ref int M() { }
Diagnostic(ErrorCode.ERR_ReturnExpected, "M").WithArguments("C.M()").WithLocation(4, 35)
);
}
}
}
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Syntax/CSharpCompilerSyntaxTest.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@
<Compile Include="Syntax\SyntaxTriviaListTests.cs" />
<Compile Include="Syntax\TrackNodeTests.cs" />
<Compile Include="TextExtensions.cs" />
<Compile Include="Parsing\ReadonlyRefReturns.cs" />
<Compile Include="Parsing\ReadonlyRefReturnsTests.cs" />
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="Resources.resx">
Expand Down
12 changes: 6 additions & 6 deletions src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class DeclarationParsingTests : ParsingTests

protected override SyntaxTree ParseTree(string text, CSharpParseOptions options)
{
return SyntaxFactory.ParseSyntaxTree(text, options ?? TestOptions.Latest);
return SyntaxFactory.ParseSyntaxTree(text, options ?? TestOptions.Regular);
}

[Fact]
Expand Down Expand Up @@ -1982,7 +1982,7 @@ public void TestDelegateWithRefReturnType()
public void TestDelegateWithReadonlyRefReturnType()
{
var text = "delegate readonly ref a b();";
var file = this.ParseFile(text, TestOptions.Latest);
var file = this.ParseFile(text, TestOptions.Regular);

Assert.NotNull(file);
Assert.Equal(1, file.Members.Count);
Expand Down Expand Up @@ -2499,7 +2499,7 @@ public void TestClassMethodWithRefReturn()
public void TestClassMethodWithReadonlyRefReturn()
{
var text = "class a { readonly ref b X() { } }";
var file = this.ParseFile(text, TestOptions.Latest);
var file = this.ParseFile(text, TestOptions.Regular);

Assert.NotNull(file);
Assert.Equal(1, file.Members.Count);
Expand Down Expand Up @@ -2573,7 +2573,7 @@ public void TestClassMethodWithRef()
public void TestClassMethodWithReadonlyRef()
{
var text = "class a { readonly ref }";
var file = this.ParseFile(text, parseOptions: TestOptions.Latest);
var file = this.ParseFile(text, parseOptions: TestOptions.Regular);

Assert.NotNull(file);
Assert.Equal(1, file.Members.Count);
Expand Down Expand Up @@ -3947,7 +3947,7 @@ public void TestClassPropertyWithRefReturn()
public void TestClassPropertyWithReadonlyRefReturn()
{
var text = "class a { readonly ref b c { get; set; } }";
var file = this.ParseFile(text, TestOptions.Latest);
var file = this.ParseFile(text, TestOptions.Regular);

Assert.NotNull(file);
Assert.Equal(1, file.Members.Count);
Expand Down Expand Up @@ -4890,7 +4890,7 @@ public void TestClassIndexerWithRefReturn()
public void TestClassIndexerWithReadonlyRefReturn()
{
var text = "class a { readonly ref b this[c d] { get; set; } }";
var file = this.ParseFile(text, TestOptions.Latest);
var file = this.ParseFile(text, TestOptions.Regular);

Assert.NotNull(file);
Assert.Equal(1, file.Members.Count);
Expand Down
Loading