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

D-declaration should use declaration expressions #14871

Merged
merged 8 commits into from
Nov 8, 2016
Merged

D-declaration should use declaration expressions #14871

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dc2fe06
WIP on syntax
jcouv Nov 1, 2016
d0f94c3
Updating binding and semantic model to new syntax
jcouv Nov 2, 2016
ecce74d
Updating editor features to adopt new syntax model
jcouv Nov 4, 2016
31e9421
Fix issue with EE scenario
jcouv Nov 5, 2016
7400a9f
PR feedback (1)
jcouv Nov 7, 2016
d5bede6
PR feedback (2)
jcouv Nov 8, 2016
7e4ea6e
Moving IsDeconstructionDeclarationLeft method
jcouv Nov 8, 2016
d39e75e
IDE feedback (1)
jcouv Nov 8, 2016
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
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,6 @@
<Compile Include="Syntax\DestructorDeclarationSyntax.cs" />
<Compile Include="Syntax\DirectiveTriviaSyntax.cs" />
<Compile Include="Syntax\ExpressionStatementSyntax.cs" />
<Compile Include="Syntax\ForStatementSyntax.cs" />
<Compile Include="Syntax\GenericNameSyntax.cs" />
<Compile Include="Syntax\CSharpSyntaxTree.DebuggerSyntaxTree.cs" />
<Compile Include="Syntax\IdentifierNameSyntax.cs" />
Expand Down
71 changes: 36 additions & 35 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 @@ -2413,8 +2413,7 @@ private MemberDeclarationSyntax ParseMemberDeclarationOrStatement(SyntaxKind par
if (deconstruction != null)
{
var semicolon = this.EatToken(SyntaxKind.SemicolonToken);
return _syntaxFactory.GlobalStatement(_syntaxFactory.DeconstructionDeclarationStatement(
new SyntaxList<SyntaxToken>(), deconstruction, semicolon));
return _syntaxFactory.GlobalStatement(_syntaxFactory.ExpressionStatement(deconstruction, semicolon));
}
}

Expand Down Expand Up @@ -7533,7 +7532,6 @@ private StatementSyntax ParseEmbeddedStatement(bool complexCheck)
case SyntaxKind.LabeledStatement:
case SyntaxKind.LocalDeclarationStatement:
case SyntaxKind.LocalFunctionStatement:
case SyntaxKind.DeconstructionDeclarationStatement:
statement = this.AddError(statement, ErrorCode.ERR_BadEmbeddedStmt);
break;
}
Expand Down Expand Up @@ -7848,7 +7846,7 @@ private ForStatementSyntax ParseForStatement()
{
// Here can be either a declaration or an expression statement list. Scan
// for a declaration first.
VariableComponentAssignmentSyntax deconstruction = null;
AssignmentExpressionSyntax deconstruction = null;
VariableDeclarationSyntax decl = null;
bool isDeclaration = false;
if (this.CurrentToken.Kind == SyntaxKind.RefKeyword)
Expand All @@ -7867,6 +7865,10 @@ private ForStatementSyntax ParseForStatement()

this.Reset(ref resetPoint);
}
else
{
initializers.Add(deconstruction);
}
}

if (isDeclaration)
Expand Down Expand Up @@ -7895,7 +7897,7 @@ private ForStatementSyntax ParseForStatement()
var closeParen = this.EatToken(SyntaxKind.CloseParenToken);
var statement = ParseEmbeddedStatement(true);

return _syntaxFactory.ForStatement(forToken, openParen, deconstruction, decl, initializers, semi, condition, semi2, incrementors, closeParen, statement);
return _syntaxFactory.ForStatement(forToken, openParen, decl, initializers, semi, condition, semi2, incrementors, closeParen, statement);
}
finally
{
Expand Down Expand Up @@ -8413,7 +8415,7 @@ private StatementSyntax ParseLocalDeclarationStatement()
if (deconstruction != null)
{
var semicolon = this.EatToken(SyntaxKind.SemicolonToken);
var result = _syntaxFactory.DeconstructionDeclarationStatement(mods.ToList(), deconstruction, semicolon);
var result = _syntaxFactory.ExpressionStatement(deconstruction, semicolon);
_pool.Free(mods);
return result;
}
Expand Down Expand Up @@ -8469,7 +8471,7 @@ private StatementSyntax ParseLocalDeclarationStatement()
/// <summary>
/// Returns null and resets the pointer if this does not look like a deconstruction-declaration after all.
/// </summary>
private VariableComponentAssignmentSyntax TryParseDeconstructionDeclarationAssignment()
private AssignmentExpressionSyntax TryParseDeconstructionDeclarationAssignment()
{
if (this.CurrentToken.Kind == SyntaxKind.OpenParenToken
|| CurrentToken.IsVarOrPredefinedType() && this.PeekToken(1).Kind == SyntaxKind.OpenParenToken)
Expand Down Expand Up @@ -8502,7 +8504,7 @@ private VariableComponentAssignmentSyntax TryParseDeconstructionDeclarationAssig
/// <summary>
/// Returns null and resets the pointer if this does not look like a deconstruction-declaration after all.
/// </summary>
private VariableComponentSyntax TryParseDeconstructionDeclaration(SyntaxKind nextExpectedKind)
private ExpressionSyntax TryParseDeconstructionDeclaration(SyntaxKind nextExpectedKind)
{
if (this.CurrentToken.Kind == SyntaxKind.OpenParenToken
|| CurrentToken.IsVarOrPredefinedType() && this.PeekToken(1).Kind == SyntaxKind.OpenParenToken)
Expand All @@ -8511,7 +8513,7 @@ private VariableComponentSyntax TryParseDeconstructionDeclaration(SyntaxKind nex

try
{
var deconstruction = ParseDeconstructionComponent(true);
var deconstruction = ParseDeconstructionDeclarationVariables(true);
if (deconstruction == null || CurrentToken.Kind != nextExpectedKind)
{
this.Reset(ref resetPoint);
Expand All @@ -8538,13 +8540,13 @@ private VariableComponentSyntax TryParseDeconstructionDeclaration(SyntaxKind nex
/// The syntax is either var form: `var (deconstruction-declaration, ...) = expression` or list form `(deconstruction-declaration, ...) = expression`.
/// Cannot return null, except at the top-level.
/// </summary>
private VariableComponentAssignmentSyntax ParseDeconstructionDeclarationAssignment()
private AssignmentExpressionSyntax ParseDeconstructionDeclarationAssignment()
{
var component = TryParseDeconstructionDeclaration(SyntaxKind.EqualsToken);
if (component == null) return null;
var equalsToken = this.EatToken(SyntaxKind.EqualsToken);
var value = this.ParseExpressionCore();
return _syntaxFactory.VariableComponentAssignment(component, equalsToken, value);
return _syntaxFactory.AssignmentExpression(SyntaxKind.SimpleAssignmentExpression, component, equalsToken, value);
}

/// <summary>
Expand All @@ -8555,7 +8557,7 @@ private VariableComponentAssignmentSyntax ParseDeconstructionDeclarationAssignme
/// Cannot return null, except at the top-level.
/// </summary>
/// <param name="topLevel">Specifies whether to parse the terminal form of a deconstruction-declaration (which can't appear at the top-level).</param>
private VariableComponentSyntax ParseDeconstructionComponent(bool topLevel = false)
private ExpressionSyntax ParseDeconstructionDeclarationVariables(bool topLevel = false)
{
if (topLevel &&
!(CurrentToken.IsVarOrPredefinedType() && this.PeekToken(1).Kind == SyntaxKind.OpenParenToken || this.CurrentToken.Kind == SyntaxKind.OpenParenToken))
Expand All @@ -8565,28 +8567,27 @@ private VariableComponentSyntax ParseDeconstructionComponent(bool topLevel = fal

// the two forms of component are
// (1) type designator
// (2) ( component ... )
VariableComponentSyntax result;
// (2) ( decl-expr, ... )
ExpressionSyntax result;
if (this.CurrentToken.Kind == SyntaxKind.OpenParenToken)
{
var openParen = this.EatToken(SyntaxKind.OpenParenToken);
var listOfComponents = _pool.AllocateSeparated<VariableComponentSyntax>();
var listOfDeclarations = _pool.AllocateSeparated<ArgumentSyntax>();
while (true)
{
listOfComponents.Add(ParseDeconstructionComponent());
listOfDeclarations.Add(_syntaxFactory.Argument(nameColon: null, refOrOutKeyword: null, expression: ParseDeconstructionDeclarationVariables()));
if (this.CurrentToken.Kind == SyntaxKind.CommaToken)
{
listOfComponents.AddSeparator(this.EatToken(SyntaxKind.CommaToken));
listOfDeclarations.AddSeparator(this.EatToken(SyntaxKind.CommaToken));
}
else
{
break;
}
}
var closeParen = this.EatToken(SyntaxKind.CloseParenToken);
SeparatedSyntaxList<VariableComponentSyntax> components = listOfComponents;
result = _syntaxFactory.ParenthesizedVariableComponent(openParen, listOfComponents, closeParen);
_pool.Free(listOfComponents);
result = _syntaxFactory.TupleExpression(openParen, listOfDeclarations, closeParen);
_pool.Free(listOfDeclarations);
}
else
{
Expand All @@ -8610,31 +8611,32 @@ private VariableComponentSyntax ParseDeconstructionComponent(bool topLevel = fal
designation = this.AddError(designation, ErrorCode.ERR_TypeExpected);
}

result = _syntaxFactory.TypedVariableComponent(type, designation);
result = _syntaxFactory.DeclarationExpression(type, designation);
}

return
topLevel ? (ComponentHasTypes(result) ? CheckFeatureAvailability(result, MessageID.IDS_FeatureTuples) : null) : result;
topLevel ? (TypeFoundInDeconstructionDeclarationVariables(result) ? CheckFeatureAvailability(result, MessageID.IDS_FeatureTuples) : null) : result;
}

private static bool ComponentHasTypes(VariableComponentSyntax node)
// Check if we can find at least one type in the deconstruction variables
private static bool TypeFoundInDeconstructionDeclarationVariables(ExpressionSyntax node)
Copy link
Contributor

@AlekseyTs AlekseyTs Nov 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

TypeFoundInDeconstructionDeclarationVariables [](start = 28, length = 45)

I am not sure if this check is relevant now. Shouldn't we always require Tuples Feature when deconstruction is used? #Closed

Copy link
Member Author

@jcouv jcouv Nov 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This is not related to the tuples features. There are two heuristics to be confident that the user typed a deconstruction declaration:

  1. we must find the expected following token (typically '=' but could be 'in' for in foreach context),
  2. the variables must include at least one specified type. That's the TypeFoundInDeconstructionDeclarationVariables method. #Resolved

{
switch (node.Kind)
{
case SyntaxKind.ParenthesizedVariableComponent:
case SyntaxKind.TupleExpression:
{
var syntax = (ParenthesizedVariableComponentSyntax)node;
if (syntax.Variables.Count <= 1) return false; // don't count 1ples
for (int i = 0; i < syntax.Variables.Count; i++)
var syntax = (TupleExpressionSyntax)node;
if (syntax.Arguments.Count <= 1) return false; // don't count 1ples
for (int i = 0; i < syntax.Arguments.Count; i++)
{
if (ComponentHasTypes(syntax.Variables[i])) return true;
if (TypeFoundInDeconstructionDeclarationVariables(syntax.Arguments[i].Expression)) return true;
}

return false;
}
case SyntaxKind.TypedVariableComponent:
case SyntaxKind.DeclarationExpression:
{
var syntax = (TypedVariableComponentSyntax)node;
var syntax = (DeclarationExpressionSyntax)node;
if (syntax.Type.IsMissing || syntax.Designation.IsMissing) return false;
Copy link
Contributor

@AlekseyTs AlekseyTs Nov 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

(syntax.Type.IsMissing || syntax.Designation.IsMissing) [](start = 27, length = 55)

Can this ever be true now? #Closed

Copy link
Member Author

@jcouv jcouv Nov 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, if we try to parse (x, var y) = e; as a d-declaration, then the type on x will be missing.
Similarly, in (int, var y) = e;, the designation will be missing. #Resolved

if (syntax.Designation.Kind == SyntaxKind.ParenthesizedVariableDesignation)
{
Expand Down Expand Up @@ -8697,7 +8699,7 @@ private bool IsPossibleDeconstructionDeclaration()
try
{
var assignment = ParseDeconstructionDeclarationAssignment();
return assignment != null && !assignment.equalsToken.IsMissing;
return assignment != null && assignment.operatorToken.Kind == SyntaxKind.EqualsToken;
}
finally
{
Expand Down Expand Up @@ -9932,10 +9934,9 @@ private ArgumentSyntax ParseArgumentExpression(bool isIndexer)
{
TypeSyntax typeSyntax = ParseType();
SyntaxToken identifier = CheckFeatureAvailability(this.ParseIdentifierToken(), MessageID.IDS_FeatureOutVar);
var declarationSyntax = _syntaxFactory.TypedVariableComponent(
typeSyntax,
_syntaxFactory.SingleVariableDesignation(identifier));
expression = _syntaxFactory.DeclarationExpression(declarationSyntax);
expression = _syntaxFactory.DeclarationExpression(
typeSyntax,
_syntaxFactory.SingleVariableDesignation(identifier));
}
else
{
Expand Down
Loading