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.

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

Draft proposed revision of syntax model for deconstruction declarations #12688

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
138 changes: 110 additions & 28 deletions src/Compilers/CSharp/Portable/Syntax/Syntax.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@
</Node>
<Node Name="DeclarationExpressionSyntax" Base="ExpressionSyntax">
<Kind Name="DeclarationExpression"/>
<Field Name="Declaration" Type="VariableDeclarationSyntax">
<Field Name="Declaration" Type="MultiVariableDeclarationSyntax">
<PropertyComment>
<summary>Declaration representing the variable declared in an out parameter.</summary>
</PropertyComment>
Expand Down Expand Up @@ -1793,6 +1793,7 @@
<Kind Name="SemicolonToken"/>
</Field>
</Node>

<Node Name="LocalDeclarationStatementSyntax" Base="StatementSyntax">
<Kind Name="LocalDeclarationStatement"/>
<Field Name="Modifiers" Type="SyntaxList&lt;SyntaxToken&gt;">
Expand All @@ -1809,26 +1810,10 @@
</Field>
</Node>

<Node Name="VariableDeconstructionDeclaratorSyntax" Base="CSharpSyntaxNode" AvoidAutoCreation="true" InternalFactory="true">
<Kind Name="VariableDeconstructionDeclarator"/>
<Field Name="OpenParenToken" Type="SyntaxToken">
<Kind Name="OpenParenToken"/>
</Field>
<Field Name="Variables" Type="SeparatedSyntaxList&lt;VariableDeclarationSyntax&gt;"/>
<Field Name="CloseParenToken" Type="SyntaxToken">
<Kind Name="CloseParenToken"/>
</Field>
<Field Name="EqualsToken" Type="SyntaxToken" Optional="true">
<Kind Name="EqualsToken"/>
</Field>
<Field Name="Value" Type="ExpressionSyntax" Optional="true"/>
</Node>

<Node Name="VariableDeclarationSyntax" Base="CSharpSyntaxNode" AvoidAutoCreation="true" InternalFactory="true">
<Node Name="VariableDeclarationSyntax" Base="CSharpSyntaxNode">
<Kind Name="VariableDeclaration"/>
<Field Name="Type" Type="TypeSyntax" Optional="true"/>
<Field Name="Type" Type="TypeSyntax"/>
<Field Name="Variables" Type="SeparatedSyntaxList&lt;VariableDeclaratorSyntax&gt;"/>
<Field Name="Deconstruction" Type="VariableDeconstructionDeclaratorSyntax" Optional="true"/>
</Node>

<Node Name="VariableDeclaratorSyntax" Base="CSharpSyntaxNode">
Expand All @@ -1852,13 +1837,71 @@
</Field>
<Field Name="Value" Type="ExpressionSyntax"/>
</Node>


<AbstractNode Name="MultiVariableDeclarationSyntax" Base="CSharpSyntaxNode">
Copy link
Member

Choose a reason for hiding this comment

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

From a naming perspective, it seems like perhaps this should just be called BaseVariableDeclarationSyntax.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not a base of VariableDeclarationSyntax.


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

</AbstractNode>
<Node Name="SimpleVariableDeclarationSyntax" Base="MultiVariableDeclarationSyntax">
<Field Name="Type" Type="TypeSyntax"/>
<Field Name="Declarator" Type="MultiVariableDeclaratorSyntax"/>
</Node>
<Node Name="TupleDeconstructionDeclarationSyntax" Base="MultiVariableDeclarationSyntax">
<Field Name="OpenParenToken" Type="SyntaxToken">
<Kind Name="OpenParenToken"/>
</Field>
<Field Name="Variables" Type="SeparatedSyntaxList&lt;MultiVariableDeclarationSyntax&gt;"/>
<Field Name="CloseParenToken" Type="SyntaxToken">
<Kind Name="CloseParenToken"/>
</Field>
</Node>

<AbstractNode Name="MultiVariableDeclaratorSyntax" Base="CSharpSyntaxNode">
</AbstractNode>
<Node Name="SingleVariableDeclaratorSyntax" Base="MultiVariableDeclaratorSyntax">
<Field Name="Identifier" Type="SyntaxToken">
<Kind Name="IdentifierToken"/>
</Field>
</Node>
<Node Name="TupleDeconstructionDeclaratorSyntax" Base="MultiVariableDeclaratorSyntax">
<Field Name="OpenParenToken" Type="SyntaxToken">
<Kind Name="OpenParenToken"/>
</Field>
<Field Name="Variables" Type="SeparatedSyntaxList&lt;MultiVariableDeclaratorSyntax&gt;"/>
<Field Name="CloseParenToken" Type="SyntaxToken">
<Kind Name="CloseParenToken"/>
</Field>
</Node>
<Node Name="WildcardVariableDeclaratorSyntax" Base="MultiVariableDeclaratorSyntax">
Copy link
Member

Choose a reason for hiding this comment

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

I'd call this IgnoredVariableDeclaratorSyntax.

<Field Name="AsteriskToken" Type="SyntaxToken">
<Kind Name="AsteriskToken"/>
</Field>
</Node>
<!-- It is possible to add additional subtypes of MultiVariableDeclaratorSyntax
to assist with error recovery. -->

<Node Name="DeconstructionDeclarationStatementSyntax" Base="StatementSyntax">
<Field Name="Modifiers" Type="SyntaxList&lt;SyntaxToken&gt;"/>
<Field Name="Assignment" Type="DeconstructionDeclarationAssignment"/>
<Field Name="SemicolonToken" Type="SyntaxToken">
<Kind Name="SemicolonToken"/>
</Field>
</Node>
<Node Name="DeconstructionDeclarationAssignment" Base="CSharpSyntaxNode">
<Field Name="Declaration" Type="MultiVariableDeclarationSyntax"/>
<Field Name="EqualsToken" Type="SyntaxToken">
<Kind Name="EqualsToken"/>
</Field>
<Field Name="Value" Type="ExpressionSyntax"/>
</Node>

<Node Name="ExpressionStatementSyntax" Base="StatementSyntax">
<Kind Name="ExpressionStatement"/>
<Field Name="Expression" Type="ExpressionSyntax"/>
<Field Name="SemicolonToken" Type="SyntaxToken">
<Kind Name="SemicolonToken"/>
</Field>
</Node>

<Node Name="EmptyStatementSyntax" Base="StatementSyntax">
<Kind Name="EmptyStatement"/>
<Field Name="SemicolonToken" Type="SyntaxToken">
Expand Down Expand Up @@ -2035,6 +2078,7 @@
<Field Name="RefKeyword" Type="SyntaxToken" Optional="true">
<Kind Name="RefKeyword"/>
</Field>
<Field Name="Deconstuction" Type="DeconstructionDeclarationAssignment" Optional="true"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Deconstruction

<Field Name="Declaration" Type="VariableDeclarationSyntax" Optional="true"/>
<Field Name="Initializers" Type="SeparatedSyntaxList&lt;ExpressionSyntax&gt;"/>
<Field Name="FirstSemicolonToken" Type="SyntaxToken">
Expand All @@ -2051,30 +2095,68 @@
<Field Name="Statement" Type="StatementSyntax"/>
</Node>

<Node Name="ForEachStatementSyntax" Base="StatementSyntax" InternalFactory="true">
<Kind Name="ForEachStatement"/>
<!-- Because there are two forms of the foreach loop, we make an abstract base. -->
<AbstractNode Name="CommonForEachStatementSyntax" Base="StatementSyntax">
<Field Name="ForEachKeyword" Type="SyntaxToken">
<Kind Name="ForEachKeyword"/>
</Field>
<Field Name="OpenParenToken" Type="SyntaxToken">
<Kind Name="OpenParenToken"/>
</Field>
<Field Name="Type" Type="TypeSyntax" Optional="true"/>
<Field Name="Identifier" Type="SyntaxToken" Optional="true">
<Field Name="InKeyword" Type="SyntaxToken">
<Kind Name="InKeyword"/>
</Field>
<Field Name="Expression" Type="ExpressionSyntax"/>
<Field Name="CloseParenToken" Type="SyntaxToken">
<Kind Name="CloseParenToken"/>
</Field>
<Field Name="Statement" Type="StatementSyntax"/>
</AbstractNode>
<Node Name="ForEachStatementSyntax" Base="CommonForEachStatementSyntax">
<!-- This is the existing C# 6 node. -->
<Kind Name="ForEachStatement"/>
<Field Name="ForEachKeyword" Type="SyntaxToken" Override="true">
<Kind Name="ForEachKeyword"/>
</Field>
<Field Name="OpenParenToken" Type="SyntaxToken" Override="true">
<Kind Name="OpenParenToken"/>
</Field>
<Field Name="Type" Type="TypeSyntax"/>
<Field Name="Identifier" Type="SyntaxToken">
<PropertyComment>
<summary>Gets the identifier.</summary>
</PropertyComment>
<Kind Name="IdentifierToken"/>
</Field>
<Field Name="DeconstructionVariables" Type="VariableDeclarationSyntax" Optional="true"/>
<Field Name="InKeyword" Type="SyntaxToken">
<Field Name="InKeyword" Type="SyntaxToken" Override="true">
<Kind Name="InKeyword"/>
</Field>
<Field Name="Expression" Type="ExpressionSyntax"/>
<Field Name="CloseParenToken" Type="SyntaxToken">
<Field Name="Expression" Type="ExpressionSyntax" Override="true"/>
<Field Name="CloseParenToken" Type="SyntaxToken" Override="true">
<Kind Name="CloseParenToken"/>
</Field>
<Field Name="Statement" Type="StatementSyntax"/>
<Field Name="Statement" Type="StatementSyntax" Override="true"/>
</Node>
<!-- We name this "DeclarationForEachStatementSyntax" because it can express existing foreach
loops. We may elect to represent all foreach loops using this node and deprecate (stop parsing
into) the old one. -->
<Node Name="MultiVariableForEachStatementSyntax" Base="CommonForEachStatementSyntax">
<Kind Name="ForEachStatement"/>
<Field Name="ForEachKeyword" Type="SyntaxToken" Override="true">
<Kind Name="ForEachKeyword"/>
</Field>
<Field Name="OpenParenToken" Type="SyntaxToken" Override="true">
<Kind Name="OpenParenToken"/>
</Field>
<Field Name="Declaration" Type="MultiVariableDeclarationSyntax"/>
<Field Name="InKeyword" Type="SyntaxToken" Override="true">
<Kind Name="InKeyword"/>
</Field>
<Field Name="Expression" Type="ExpressionSyntax" Override="true"/>
<Field Name="CloseParenToken" Type="SyntaxToken" Override="true">
<Kind Name="CloseParenToken"/>
</Field>
<Field Name="Statement" Type="StatementSyntax" Override="true"/>
</Node>

<Node Name="UsingStatementSyntax" Base="StatementSyntax">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,6 @@ protected static string OverrideOrNewModifier(Field field)
return IsOverride(field) ? "override " : IsNew(field) ? "new " : "";
}

protected static string AccessibilityModifier(Field field)
{
return IsInternal(field) ? "internal" : "public";
}

protected static bool CanBeField(Field field)
{
return field.Type != "SyntaxToken" && !IsAnyList(field.Type) && !IsOverride(field) && !IsNew(field);
Expand Down Expand Up @@ -209,11 +204,6 @@ protected static bool IsOverride(Field f)
return f.Override != null && string.Compare(f.Override, "true", true) == 0;
}

protected static bool IsInternal(Field f)
{
return f.Internal != null && string.Compare(f.Internal, "true", true) == 0;
}

protected static bool IsNew(Field f)
{
return f.New != null && string.Compare(f.New, "true", true) == 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ public class Field
[XmlAttribute]
public string New;

[XmlAttribute]
public string Internal;

[XmlElement(ElementName = "Kind", Type = typeof(Kind))]
public List<Kind> Kinds;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,6 @@ public class Node : TreeType
[XmlAttribute]
public string Errors;

/// <summary>
/// Even if the Node only has optional or struct fields, don't treat it as AutoCreatable.
/// Don't introduce a factory method with no arguments.
/// </summary>
[XmlAttribute]
public bool AvoidAutoCreation = false;

/// <summary>
/// The factory method with all the fields should be internal. The corresponding Update method as well.
/// Other factory methods are not generated and have to be written by hand.
/// </summary>
[XmlAttribute]
public bool InternalFactory = false;

[XmlElement(ElementName = "Kind", Type = typeof(Kind))]
public List<Kind> Kinds;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ private void WriteRedType(TreeType node)
var fieldType = field.Type == "SyntaxList<SyntaxToken>" ? "SyntaxTokenList" : field.Type;
WriteLine();
WriteComment(field.PropertyComment, " ");
WriteLine(" {0} abstract {1}{2} {3} {{ get; }}", AccessibilityModifier(field), (IsNew(field) ? "new " : ""), fieldType, field.Name);
WriteLine(" {0} abstract {1}{2} {3} {{ get; }}", "public", (IsNew(field) ? "new " : ""), fieldType, field.Name);
}
}

Expand All @@ -916,7 +916,7 @@ private void WriteRedType(TreeType node)
var field = valueFields[i];
WriteLine();
WriteComment(field.PropertyComment, " ");
WriteLine(" {0} abstract {1}{2} {3} {{ get; }}", AccessibilityModifier(field), (IsNew(field) ? "new " : ""), field.Type, field.Name);
WriteLine(" {0} abstract {1}{2} {3} {{ get; }}", "public", (IsNew(field) ? "new " : ""), field.Type, field.Name);
}

WriteLine(" }");
Expand Down Expand Up @@ -964,7 +964,7 @@ private void WriteRedType(TreeType node)
if (field.Type == "SyntaxToken")
{
WriteComment(field.PropertyComment, " ");
WriteLine(" {0} {1}{2} {3} ", AccessibilityModifier(field), OverrideOrNewModifier(field), field.Type, field.Name);
WriteLine(" {0} {1}{2} {3} ", "public", OverrideOrNewModifier(field), field.Type, field.Name);
WriteLine(" {");
if (IsOptional(field))
{
Expand All @@ -986,7 +986,7 @@ private void WriteRedType(TreeType node)
else if (field.Type == "SyntaxList<SyntaxToken>")
{
WriteComment(field.PropertyComment, " ");
WriteLine(" {0} {1}SyntaxTokenList {2} ", AccessibilityModifier(field), OverrideOrNewModifier(field), field.Name);
WriteLine(" {0} {1}SyntaxTokenList {2} ", "public", OverrideOrNewModifier(field), field.Name);
WriteLine(" {");
WriteLine(" get");
WriteLine(" {");
Expand All @@ -1001,7 +1001,7 @@ private void WriteRedType(TreeType node)
else
{
WriteComment(field.PropertyComment, " ");
WriteLine(" {0} {1}{2} {3} ", AccessibilityModifier(field), OverrideOrNewModifier(field), field.Type, field.Name);
WriteLine(" {0} {1}{2} {3} ", "public", OverrideOrNewModifier(field), field.Type, field.Name);
WriteLine(" {");
WriteLine(" get");
WriteLine(" {");
Expand Down Expand Up @@ -1044,7 +1044,7 @@ private void WriteRedType(TreeType node)
var field = valueFields[i];
WriteComment(field.PropertyComment, " ");
WriteLine(" {0} {1}{2} {3} {{ get {{ return ((Microsoft.CodeAnalysis.CSharp.Syntax.InternalSyntax.{4})this.Green).{3}; }} }}",
AccessibilityModifier(field), OverrideOrNewModifier(field), field.Type, field.Name, node.Name
"public", OverrideOrNewModifier(field), field.Type, field.Name, node.Name
);
WriteLine();
}
Expand Down Expand Up @@ -1184,7 +1184,7 @@ private void WriteRedVisitor(bool genericArgument, bool genericResult)
private void WriteRedUpdateMethod(Node node)
{
WriteLine();
Write(" {0} {1} Update(", (node.Fields.Any(f => IsInternal(f)) || node.InternalFactory) ? "internal" : "public", node.Name);
Write(" {0} {1} Update(", "public", node.Name);

// parameters
for (int f = 0; f < node.Fields.Count; f++)
Expand Down Expand Up @@ -1283,7 +1283,7 @@ private void WriteRedSetters(Node node)
var type = this.GetRedPropertyType(field);

WriteLine();
WriteLine(" {0} {1} With{2}({3} {4})", AccessibilityModifier(field), node.Name, StripPost(field.Name, "Opt"), type, CamelCase(field.Name));
WriteLine(" {0} {1} With{2}({3} {4})", "public", node.Name, StripPost(field.Name, "Opt"), type, CamelCase(field.Name));
WriteLine(" {");

// call update inside each setter
Expand Down Expand Up @@ -1322,7 +1322,7 @@ private void WriteRedListHelperMethods(Node node)
else
{
Node referencedNode = GetNode(field.Type);
if (referencedNode != null && !referencedNode.AvoidAutoCreation && (!IsOptional(field) || RequiredFactoryArgumentCount(referencedNode) == 0))
if (referencedNode != null && (!IsOptional(field) || RequiredFactoryArgumentCount(referencedNode) == 0))
{
// look for list members...
for (int rf = 0; rf < referencedNode.Fields.Count; rf++)
Expand Down Expand Up @@ -1448,14 +1448,10 @@ private void WriteRedFactories()
{
var node = nodes[i];
this.WriteRedFactory(node);

if (!node.InternalFactory)
{
this.WriteRedFactoryWithNoAutoCreatableTokens(node);
this.WriteRedMinimalFactory(node);
this.WriteRedMinimalFactory(node, withStringNames: true);
this.WriteKindConverters(node);
}
this.WriteRedFactoryWithNoAutoCreatableTokens(node);
this.WriteRedMinimalFactory(node);
this.WriteRedMinimalFactory(node, withStringNames: true);
this.WriteKindConverters(node);
}

WriteLine(" }");
Expand All @@ -1476,7 +1472,7 @@ private bool IsAutoCreatableToken(Node node, Field field)
private bool IsAutoCreatableNode(Node node, Field field)
{
var referencedNode = GetNode(field.Type);
return (referencedNode != null && !referencedNode.AvoidAutoCreation && RequiredFactoryArgumentCount(referencedNode) == 0);
return (referencedNode != null && RequiredFactoryArgumentCount(referencedNode) == 0);
}

private bool IsRequiredFactoryField(Node node, Field field)
Expand Down Expand Up @@ -1536,7 +1532,7 @@ private void WriteRedFactory(Node nd)

WriteComment(string.Format("<summary>Creates a new {0} instance.</summary>", nd.Name), " ");

Write(" {0} static {1} {2}(", (nd.Fields.Any(f => IsInternal(f)) || nd.InternalFactory) ? "internal" : "public", nd.Name, StripPost(nd.Name, "Syntax"));
Write(" {0} static {1} {2}(", "public", nd.Name, StripPost(nd.Name, "Syntax"));
WriteRedFactoryParameters(nd);

WriteLine(")");
Expand Down Expand Up @@ -1746,7 +1742,7 @@ private void WriteRedFactoryWithNoAutoCreatableTokens(Node nd)
this.WriteLine();

WriteComment(string.Format("<summary>Creates a new {0} instance.</summary>", nd.Name), " ");
Write(" {0} static {1} {2}(", factoryWithNoAutoCreatableTokenFields.Any(f => IsInternal(f)) ? "internal" : "public", nd.Name, StripPost(nd.Name, "Syntax"));
Write(" {0} static {1} {2}(", "public", nd.Name, StripPost(nd.Name, "Syntax"));

bool hasPreviousParameter = false;
if (nd.Kinds.Count > 1)
Expand Down Expand Up @@ -1867,7 +1863,7 @@ private void WriteRedMinimalFactory(Node nd, bool withStringNames = false)
this.WriteLine();

WriteComment(string.Format("<summary>Creates a new {0} instance.</summary>", nd.Name), " ");
Write(" {0} static {1} {2}(", minimalFactoryfields.Any(f => IsInternal(f)) ? "internal" : "public", nd.Name, StripPost(nd.Name, "Syntax"));
Write(" {0} static {1} {2}(", "public", nd.Name, StripPost(nd.Name, "Syntax"));

bool hasPreviousParameter = false;
if (nd.Kinds.Count > 1)
Expand Down