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

Improve error tolerance when parsing invalid implicit arrays. #24706

Merged
merged 2 commits into from
Feb 9, 2018
Merged

Improve error tolerance when parsing invalid implicit arrays. #24706

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
44e0387
Improve error tolerance when parsing invalid implicit arrays.
CyrusNajmabadi Feb 8, 2018
da6fa53
Be resilient to infinite loops.
CyrusNajmabadi Feb 9, 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
39 changes: 29 additions & 10 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 @@ -450,19 +450,19 @@ private void AddSkippedNamespaceText(
{
if (body.Members.Count > 0)
{
body.Members[body.Members.Count - 1] = AddTrailingSkippedSyntax(body.Members[body.Members.Count - 1], skippedSyntax);
AddTrailingSkippedSyntax(body.Members, skippedSyntax);
}
else if (body.Attributes.Count > 0)
{
body.Attributes[body.Attributes.Count - 1] = AddTrailingSkippedSyntax(body.Attributes[body.Attributes.Count - 1], skippedSyntax);
AddTrailingSkippedSyntax(body.Attributes, skippedSyntax);
}
else if (body.Usings.Count > 0)
{
body.Usings[body.Usings.Count - 1] = AddTrailingSkippedSyntax(body.Usings[body.Usings.Count - 1], skippedSyntax);
AddTrailingSkippedSyntax(body.Usings, skippedSyntax);
}
else if (body.Externs.Count > 0)
{
body.Externs[body.Externs.Count - 1] = AddTrailingSkippedSyntax(body.Externs[body.Externs.Count - 1], skippedSyntax);
AddTrailingSkippedSyntax(body.Externs, skippedSyntax);
}
else if (openBrace != null)
{
Expand Down Expand Up @@ -3275,7 +3275,7 @@ private PostSkipAction SkipBadListTokensWithExpectedKindHelper(
var action = SkipBadTokensWithExpectedKind(isNotExpectedFunction, abortFunction, expected, out lastItemTrailingTrivia);
if (lastItemTrailingTrivia != null)
{
list[list.Count - 1] = AddTrailingSkippedSyntax((CSharpSyntaxNode)list[list.Count - 1], lastItemTrailingTrivia);
AddTrailingSkippedSyntax(list, lastItemTrailingTrivia);
}
trailingTrivia = null;
return action;
Expand All @@ -3299,7 +3299,7 @@ private PostSkipAction SkipBadListTokensWithErrorCodeHelper<TNode>(
var action = SkipBadTokensWithErrorCode(isNotExpectedFunction, abortFunction, error, out lastItemTrailingTrivia);
if (lastItemTrailingTrivia != null)
{
list[list.Count - 1] = AddTrailingSkippedSyntax(list[list.Count - 1], lastItemTrailingTrivia);
AddTrailingSkippedSyntax(list, lastItemTrailingTrivia);
}
trailingTrivia = null;
return action;
Expand Down Expand Up @@ -10600,13 +10600,32 @@ private ImplicitArrayCreationExpressionSyntax ParseImplicitlyTypedArrayCreation(
var commas = _pool.Allocate();
try
{
while (this.CurrentToken.Kind == SyntaxKind.CommaToken)
int lastTokenPosition = -1;
while (IsMakingProgress(ref lastTokenPosition))
{
commas.Add(this.EatToken());
if (this.IsPossibleExpression())
{
var size = this.AddError(this.ParseExpressionCore(), ErrorCode.ERR_InvalidArray);
Copy link
Contributor

@alrz alrz Feb 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

💯 If you don't mind I incorporate this line in the stackalloc inits PR

Copy link
Member Author

Choose a reason for hiding this comment

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

i would mind that greatly. roslyn is built on non-cooperation, and it should stay that way.

if (commas.Count == 0)
{
openBracket = AddTrailingSkippedSyntax(openBracket, size);
}
else
{
AddTrailingSkippedSyntax(commas, size);
}
}

if (this.CurrentToken.Kind == SyntaxKind.CommaToken)
{
commas.Add(this.EatToken());
continue;
}

break;
}

var closeBracket = this.EatToken(SyntaxKind.CloseBracketToken);

var initializer = this.ParseArrayInitializer();

return _syntaxFactory.ImplicitArrayCreationExpression(@new, openBracket, commas.ToList(), closeBracket, initializer);
Expand Down Expand Up @@ -10816,7 +10835,7 @@ private ParameterListSyntax ParseLambdaParameterList()

// additional parameters
int tokenProgress = -1;
while(IsMakingProgress(ref tokenProgress))
while (IsMakingProgress(ref tokenProgress))
{
if (this.CurrentToken.Kind == SyntaxKind.CloseParenToken)
{
Expand Down
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,16 @@ protected TNode AddLeadingSkippedSyntax<TNode>(TNode node, GreenNode skippedSynt
return SyntaxFirstTokenReplacer.Replace(node, oldToken, newToken, skippedSyntax.FullWidth);
}

protected void AddTrailingSkippedSyntax(SyntaxListBuilder list, GreenNode skippedSyntax)
{
list[list.Count - 1] = AddTrailingSkippedSyntax((CSharpSyntaxNode)list[list.Count - 1], skippedSyntax);
}

protected void AddTrailingSkippedSyntax<TNode>(SyntaxListBuilder<TNode> list, GreenNode skippedSyntax) where TNode : CSharpSyntaxNode
{
list[list.Count - 1] = AddTrailingSkippedSyntax(list[list.Count - 1], skippedSyntax);
}

protected TNode AddTrailingSkippedSyntax<TNode>(TNode node, GreenNode skippedSyntax) where TNode : CSharpSyntaxNode
{
var token = node as SyntaxToken;
Expand Down
25 changes: 12 additions & 13 deletions src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_ArrayCreationAndInitializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,27 +435,26 @@ public void F(int dimension)
}
";
string expectedOperationTree = @"
IArrayCreationOperation (OperationKind.ArrayCreation, Type: System.Int32[], IsInvalid) (Syntax: 'new[2]/*</bind>*/')
Copy link
Member Author

Choose a reason for hiding this comment

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

this used to be the case because we'd parse "new [2]" as "new [] { 2 }" (with lots of missing tokens). The new parse is "new [] { }" (with '2' as skipped text), so we get semantic changes as well.

IArrayCreationOperation (OperationKind.ArrayCreation, Type: ?[], IsInvalid) (Syntax: 'new[2]/*</bind>*/')
Dimension Sizes(1):
ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid, IsImplicit) (Syntax: 'new[2]/*</bind>*/')
ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0, IsInvalid, IsImplicit) (Syntax: 'new[2]/*</bind>*/')
Initializer:
IArrayInitializerOperation (1 elements) (OperationKind.ArrayInitializer, Type: null, IsInvalid) (Syntax: '2]/*</bind>*/')
Element Values(1):
ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2, IsInvalid) (Syntax: '2')
IArrayInitializerOperation (0 elements) (OperationKind.ArrayInitializer, Type: null, IsInvalid) (Syntax: '')
Element Values(0)
";
var expectedDiagnostics = new DiagnosticDescription[] {
// CS1003: Syntax error, ']' expected
// file.cs(6,31): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = /*<bind>*/new[2]/*</bind>*/;
Diagnostic(ErrorCode.ERR_SyntaxError, "2").WithArguments("]", "").WithLocation(6, 31),
// CS1514: { expected
Diagnostic(ErrorCode.ERR_InvalidArray, "2").WithLocation(6, 31),
// file.cs(6,44): error CS1514: { expected
// var x = /*<bind>*/new[2]/*</bind>*/;
Diagnostic(ErrorCode.ERR_LbraceExpected, "2").WithLocation(6, 31),
// CS1003: Syntax error, ',' expected
Diagnostic(ErrorCode.ERR_LbraceExpected, ";").WithLocation(6, 44),
// file.cs(6,44): error CS1513: } expected
// var x = /*<bind>*/new[2]/*</bind>*/;
Diagnostic(ErrorCode.ERR_SyntaxError, "]").WithArguments(",", "]").WithLocation(6, 32),
// CS1513: } expected
Diagnostic(ErrorCode.ERR_RbraceExpected, ";").WithLocation(6, 44),
// file.cs(6,27): error CS0826: No best type found for implicitly-typed array
// var x = /*<bind>*/new[2]/*</bind>*/;
Diagnostic(ErrorCode.ERR_RbraceExpected, ";").WithLocation(6, 44)
Diagnostic(ErrorCode.ERR_ImplicitlyTypedArrayNoBestType, "new[2]/*</bind>*/").WithLocation(6, 27)
};

VerifyOperationTreeAndDiagnosticsForTest<ImplicitArrayCreationExpressionSyntax>(source, expectedOperationTree, expectedDiagnostics);
Expand Down
199 changes: 199 additions & 0 deletions src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,205 @@ public static int Main()
Diagnostic(ErrorCode.ERR_SyntaxError, ";").WithArguments("]", ";"));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray1()
{
var test = @"
class C {
void Goo() {
var x = new[3] { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,21): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[3] { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray2()
{
var test = @"
class C {
void Goo() {
var x = new[3,] { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,21): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[3,] { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray3()
{
var test = @"
class C {
void Goo() {
var x = new[,3] { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,22): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[,3] { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 22));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray4()
{
var test = @"
class C {
void Goo() {
var x = new[,3 { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,22): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[,3 { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 22),
// (4,24): error CS1003: Syntax error, ']' expected
// var x = new[,3 { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments("]", "{").WithLocation(4, 24));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray5()
{
var test = @"
class C {
void Goo() {
var x = new[3 { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,21): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[3 { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21),
// (4,23): error CS1003: Syntax error, ']' expected
// var x = new[3 { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments("]", "{").WithLocation(4, 23));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray6()
{
var test = @"
class C {
void Goo() {
var x = new[3, { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,21): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[3, { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21),
// (4,24): error CS1003: Syntax error, ']' expected
// var x = new[3, { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments("]", "{").WithLocation(4, 24));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray7()
{
var test = @"
class C {
void Goo() {
var x = new[3,,] { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,21): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[3,,] { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray8()
{
var test = @"
class C {
void Goo() {
var x = new[,3,] { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,22): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[,3,] { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 22));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray9()
{
var test = @"
class C {
void Goo() {
var x = new[,,3] { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,23): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[,,3] { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 23));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray10()
{
var test = @"
class C {
void Goo() {
var x = new[3,,3] { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,21): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[3,,3] { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21),
// (4,24): error CS0178: Invalid rank specifier: expected ',' or ']'
// var x = new[3,,3] { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 24));
}

[Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")]
public void CS0178ERR_InvalidArray_ImplicitArray11()
{
var test = @"
class C {
void Goo() {
var x = new[ { 1, 2, 3 };
}
}
";

ParseAndValidate(test,
// (4,22): error CS1003: Syntax error, ']' expected
// var x = new[ { 1, 2, 3 };
Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments("]", "{").WithLocation(4, 22));
}

[Fact]
public void CS0230ERR_BadForeachDecl()
{
Expand Down