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

Do not allow a let-statement as an embedded statement. #9216

Merged
merged 1 commit into from
Feb 26, 2016
Merged

Do not allow a let-statement as an embedded statement. #9216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 34 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/PatternVariableFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,34 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Roslyn.Utilities;
using System.Collections.Generic;
using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp
{
class PatternVariableFinder : CSharpSyntaxWalker
{
ArrayBuilder<DeclarationPatternSyntax> declarationPatterns = ArrayBuilder<DeclarationPatternSyntax>.GetInstance();
ArrayBuilder<DeclarationPatternSyntax> declarationPatterns;
ArrayBuilder<ExpressionSyntax> expressionsToVisit = ArrayBuilder<ExpressionSyntax>.GetInstance();
internal static ArrayBuilder<DeclarationPatternSyntax> FindPatternVariables(
ExpressionSyntax expression = null,
ImmutableArray<ExpressionSyntax> expressions = default(ImmutableArray<ExpressionSyntax>),
ImmutableArray<PatternSyntax> patterns = default(ImmutableArray<PatternSyntax>))
{
var finder = s_poolInstance.Allocate();
finder.declarationPatterns = ArrayBuilder<DeclarationPatternSyntax>.GetInstance();
finder.Visit(expression);

// push expressions to be visited onto a stack
var expressionsToVisit = finder.expressionsToVisit;
if (expression != null) expressionsToVisit.Add(expression);
if (!expressions.IsDefaultOrEmpty)
{
foreach (var subExpression in expressions)
{
if (subExpression != null) finder.Visit(subExpression);
if (subExpression != null) expressionsToVisit.Add(subExpression);
}
}
finder.VisitExpressions();

if (!patterns.IsDefaultOrEmpty)
{
foreach (var pattern in patterns)
Expand All @@ -37,10 +44,21 @@ internal static ArrayBuilder<DeclarationPatternSyntax> FindPatternVariables(

var result = finder.declarationPatterns;
finder.declarationPatterns = null;
Debug.Assert(finder.expressionsToVisit.Count == 0);
s_poolInstance.Free(finder);
return result;
}

private void VisitExpressions()
{
while (expressionsToVisit.Count != 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is doesn't preserve the order in which expressions are visited, which leads to a wrong behavior when duplicate declarations are involved. Please fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

See how it was done for declaration expressions in LocalScopeBinder.cs.
The implementation of VisitBinaryExpression of interest was removed in 9d36a64a.

{
var e = expressionsToVisit[expressionsToVisit.Count - 1];
expressionsToVisit.RemoveLast();
Visit(e);
}
}

public override void VisitDeclarationPattern(DeclarationPatternSyntax node)
{
declarationPatterns.Add(node);
Expand All @@ -50,6 +68,19 @@ public override void VisitParenthesizedLambdaExpression(ParenthesizedLambdaExpre
public override void VisitSimpleLambdaExpression(SimpleLambdaExpressionSyntax node) { }
public override void VisitAnonymousMethodExpression(AnonymousMethodExpressionSyntax node) { }
public override void VisitQueryExpression(QueryExpressionSyntax node) { }
public override void VisitBinaryExpression(BinaryExpressionSyntax node)
{
expressionsToVisit.Add(node.Left);
expressionsToVisit.Add(node.Right);
}
public override void VisitPostfixUnaryExpression(PostfixUnaryExpressionSyntax node)
{
expressionsToVisit.Add(node.Operand);
}
public override void VisitPrefixUnaryExpression(PrefixUnaryExpressionSyntax node)
{
expressionsToVisit.Add(node.Operand);
}
public override void VisitMatchExpression(MatchExpressionSyntax node)
{
Visit(node.Left);
Expand Down
9 changes: 7 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 @@ -7248,9 +7248,14 @@ private StatementSyntax ParseEmbeddedStatement(bool complexCheck)
// An "embedded" statement is simply a statement that is not a labelled
// statement or a declaration statement. Parse a normal statement and post-
// check for the error case.
if (statement != null && (statement.Kind == SyntaxKind.LabeledStatement || statement.Kind == SyntaxKind.LocalDeclarationStatement))
switch (statement?.Kind)
{
statement = this.AddError(statement, ErrorCode.ERR_BadEmbeddedStmt);
case SyntaxKind.LabeledStatement:
case SyntaxKind.LocalDeclarationStatement:
case SyntaxKind.LocalFunctionStatement:
case SyntaxKind.LetStatement:
statement = this.AddError(statement, ErrorCode.ERR_BadEmbeddedStmt);
break;
}

return statement;
Expand Down
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,9 @@ void Test18()
// (165,13): error CS1023: Embedded statement cannot be a declaration or labeled statement
// var x18 = 11;
Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "var x18 = 11;").WithLocation(165, 13),
// (168,13): error CS1023: Embedded statement cannot be a declaration or labeled statement
// let y18 = 11;
Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "let y18 = 11;").WithLocation(168, 13),
// (16,17): error CS0841: Cannot use local variable 'x2' before it is declared
// var y = x2;
Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x2").WithArguments("x2").WithLocation(16, 17),
Expand Down Expand Up @@ -1617,6 +1620,9 @@ void Test18()
// (165,13): error CS1023: Embedded statement cannot be a declaration or labeled statement
// var x18 = 11;
Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "var x18 = 11;").WithLocation(165, 13),
// (168,13): error CS1023: Embedded statement cannot be a declaration or labeled statement
// let var y18 = 11;
Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "let var y18 = 11;").WithLocation(168, 13),
// (154,33): error CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code
// void Test16(int x16) => let var x16 = 11;
Diagnostic(ErrorCode.ERR_TypeVarNotFound, "var").WithLocation(154, 33),
Expand Down Expand Up @@ -1897,6 +1903,9 @@ class C1
var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe, parseOptions: patternParseOptions.WithLocalFunctionsFeature());

compilation.VerifyDiagnostics(
// (83,18): error CS1023: Embedded statement cannot be a declaration or labeled statement
// else let z9 = x9 when z9 is true else System.Console.WriteLine();
Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "let z9 = x9 when z9 is true else System.Console.WriteLine();").WithLocation(83, 18),
// (12,42): error CS0165: Use of unassigned local variable 'y1'
// else throw (System.Exception)y1;
Diagnostic(ErrorCode.ERR_UseDefViolation, "y1").WithArguments("y1").WithLocation(12, 42),
Expand Down Expand Up @@ -2595,6 +2604,9 @@ void Test14()
// (110,13): error CS1023: Embedded statement cannot be a declaration or labeled statement
// var y12 = 12;
Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "var y12 = 12;").WithLocation(110, 13),
// (116,13): error CS1023: Embedded statement cannot be a declaration or labeled statement
// let y13 = 12;
Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "let y13 = 12;").WithLocation(116, 13),
// (18,38): error CS0103: The name 'x1' does not exist in the current context
// System.Console.WriteLine(x1);
Diagnostic(ErrorCode.ERR_NameNotInContext, "x1").WithArguments("x1").WithLocation(18, 38),
Expand Down