Skip to content

Commit

Permalink
Added initial support for out vars.
Browse files Browse the repository at this point in the history
- Feature flag.
- Syntax model changes.
- Parsing.
- Binding (for explicitly typed variables only).
- Flow analysis.
- SemanticModel – added new API GetDeclaredSymbol(ArgumentSyntax) to return symbol for the declared local.

Basic scenarios work end to end.
  • Loading branch information
AlekseyTs committed May 18, 2016
1 parent 1425b76 commit 68aca6f
Show file tree
Hide file tree
Showing 28 changed files with 1,229 additions and 41 deletions.
38 changes: 38 additions & 0 deletions docs/features/outvar.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,41 @@ An *out variable* may not be referenced before the close parenthesis of the invo
```

> **Note**: There is a discussion thread for this feature at https://github.com/dotnet/roslyn/issues/6183

**ArgumentSyntax node is extended as follows to accommodate for the new syntax:**

- ```Expression``` field is made optional.
- Two new optional fields are added.
```
<Field Name="Type" Type="TypeSyntax" Optional="true"/>
<Field Name="Identifier" Type="SyntaxToken" Optional="true">
```


**SemanticModel changes:**

Added new API
```
/// <summary>
/// Given an argument syntax, get symbol for an out var that it declares, if any.
/// </summary>
/// <param name="declarationSyntax">The syntax node that declares a variable.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The symbol that was declared.</returns>
public ISymbol GetDeclaredSymbol(ArgumentSyntax declarationSyntax, CancellationToken cancellationToken = default(CancellationToken));
```


**Open issues:**

- Syntax model is still in flux.
- Need to get confirmation from LDM that we really want to make these variables read-only. For now they are just regular, writable, variables.
- Need to get confirmation from LDM that we really want to disallow referencing out variables the declaring argument list. It seems nothing prevents us from allowing such references for explicitly typed variables. Allowing them for now.


**TODO:**

[ ] Add tests for scope rules. Given that currently scoping rules match the rules for pattern variables, and implementation takes advantage of existing infrastructure added for pattern variables, the priority of adding these tests is low. We have pretty good suite of tests for pattern variables.

[ ] Need to get an approval for the new SemanticModel.GetDeclaredSymbol API.
5 changes: 2 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,9 @@ private AnalyzedAttributeArguments BindAttributeArguments(AttributeArgumentListS
diagnostics,
hadError,
argument,
argument.Expression,
BindArgumentExpression(diagnostics, argument.Expression, RefKind.None, allowArglist: false),
argument.NameColon,
refKind: RefKind.None,
allowArglist: false);
refKind: RefKind.None);

if (boundNamedArgumentsBuilder != null)
{
Expand Down
98 changes: 76 additions & 22 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2023,6 +2023,11 @@ private bool RefMustBeObeyed(bool isDelegateCreation, ArgumentSyntax argumentSyn
return true;
}

if (argumentSyntax.Expression == null)
{
return true;
}

switch (argumentSyntax.Expression.Kind())
{
// The next 3 cases should never be allowed as they cannot be ref/out. Assuming a bug in legacy compiler.
Expand Down Expand Up @@ -2056,15 +2061,16 @@ private bool RefMustBeObeyed(bool isDelegateCreation, ArgumentSyntax argumentSyn
// Note: Some others should still be rejected when ref/out present. See RefMustBeObeyed.
RefKind refKind = origRefKind == RefKind.None || RefMustBeObeyed(isDelegateCreation, argumentSyntax) ? origRefKind : RefKind.None;

BoundExpression boundArgument = BindArgumentValue(diagnostics, argumentSyntax, allowArglist, refKind);

hadError |= BindArgumentAndName(
result,
diagnostics,
hadError,
argumentSyntax,
argumentSyntax.Expression,
boundArgument,
argumentSyntax.NameColon,
refKind,
allowArglist);
refKind);

// check for ref/out property/indexer, only needed for 1 parameter version
if (!hadError && isDelegateCreation && origRefKind != RefKind.None && result.Arguments.Count == 1)
Expand All @@ -2081,6 +2087,48 @@ private bool RefMustBeObeyed(bool isDelegateCreation, ArgumentSyntax argumentSyn
return hadError;
}

private BoundExpression BindArgumentValue(DiagnosticBag diagnostics, ArgumentSyntax argumentSyntax, bool allowArglist, RefKind refKind)
{
if (argumentSyntax.Expression != null)
{
return BindArgumentExpression(diagnostics, argumentSyntax.Expression, refKind, allowArglist);
}

var typeSyntax = argumentSyntax.Type;

bool isConst = false;
bool isVar;
AliasSymbol alias;
TypeSymbol declType = BindVariableType(argumentSyntax, diagnostics, typeSyntax, ref isConst, out isVar, out alias);

SourceLocalSymbol localSymbol = this.LookupLocal(argumentSyntax.Identifier);

// In error scenarios with misplaced code, it is possible we can't bind the local declaration.
// This occurs through the semantic model. In that case concoct a plausible result.
if ((object)localSymbol == null)
{
localSymbol = SourceLocalSymbol.MakeLocal(
ContainingMemberOrLambda,
this,
RefKind.None,
typeSyntax,
argumentSyntax.Identifier,
LocalDeclarationKind.RegularVariable,
initializer: null);
}
else
{
this.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics);
}

if (isVar)
{
// PROTOTYPE(outvar):
throw new NotImplementedException();
}

return new BoundLocal(argumentSyntax, localSymbol, constantValueOpt: null, type: declType);
}

// Bind a named/positional argument.
// Prevent cascading diagnostic by considering the previous
Expand All @@ -2090,26 +2138,12 @@ private bool RefMustBeObeyed(bool isDelegateCreation, ArgumentSyntax argumentSyn
DiagnosticBag diagnostics,
bool hadError,
CSharpSyntaxNode argumentSyntax,
ExpressionSyntax argumentExpression,
BoundExpression boundArgumentExpression,
NameColonSyntax nameColonSyntax,
RefKind refKind,
bool allowArglist)
RefKind refKind)
{
Debug.Assert(argumentSyntax is ArgumentSyntax || argumentSyntax is AttributeArgumentSyntax);

BindValueKind valueKind = refKind == RefKind.None ? BindValueKind.RValue : BindValueKind.RefOrOut;

// Bind argument and verify argument matches rvalue or out param requirements.
BoundExpression argument;
if (allowArglist)
{
argument = this.BindValueAllowArgList(argumentExpression, diagnostics, valueKind);
}
else
{
argument = this.BindValue(argumentExpression, diagnostics, valueKind);
}

bool hasRefKinds = result.RefKinds.Any();
if (refKind != RefKind.None)
{
Expand Down Expand Up @@ -2171,7 +2205,7 @@ private bool RefMustBeObeyed(bool isDelegateCreation, ArgumentSyntax argumentSyn
hadError = true;
}

argument = ToBadExpression(argument);
boundArgumentExpression = ToBadExpression(boundArgumentExpression);
}

result.Names.Add(nameColonSyntax.Name);
Expand All @@ -2186,16 +2220,36 @@ private bool RefMustBeObeyed(bool isDelegateCreation, ArgumentSyntax argumentSyn
hadError = true;
}

argument = ToBadExpression(argument);
boundArgumentExpression = ToBadExpression(boundArgumentExpression);

result.Names.Add(null);
}

result.Arguments.Add(argument);
result.Arguments.Add(boundArgumentExpression);

return hadError;
}

/// <summary>
/// Bind argument and verify argument matches rvalue or out param requirements.
/// </summary>
private BoundExpression BindArgumentExpression(DiagnosticBag diagnostics, ExpressionSyntax argumentExpression, RefKind refKind, bool allowArglist)
{
BindValueKind valueKind = refKind == RefKind.None ? BindValueKind.RValue : BindValueKind.RefOrOut;

BoundExpression argument;
if (allowArglist)
{
argument = this.BindValueAllowArgList(argumentExpression, diagnostics, valueKind);
}
else
{
argument = this.BindValue(argumentExpression, diagnostics, valueKind);
}

return argument;
}

private void CoerceArguments<TMember>(
MemberResolutionResult<TMember> methodResult,
ArrayBuilder<BoundExpression> arguments,
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ private BoundStatement BindDeclarationStatementParts(LocalDeclarationStatementSy

private TypeSymbol BindVariableType(CSharpSyntaxNode declarationNode, DiagnosticBag diagnostics, TypeSyntax typeSyntax, ref bool isConst, out bool isVar, out AliasSymbol alias)
{
Debug.Assert(declarationNode.Kind() == SyntaxKind.LocalDeclarationStatement);
Debug.Assert(declarationNode.Kind() == SyntaxKind.LocalDeclarationStatement || declarationNode.Kind() == SyntaxKind.Argument);

// If the type is "var" then suppress errors when binding it. "var" might be a legal type
// or it might not; if it is not then we do not want to report an error. If it is, then
Expand Down
23 changes: 17 additions & 6 deletions src/Compilers/CSharp/Portable/Binder/PatternVariableFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.CSharp
internal class PatternVariableFinder : CSharpSyntaxWalker
{
private Binder _binder;
private ArrayBuilder<LocalSymbol> _declarationPatterns;
private ArrayBuilder<LocalSymbol> _localsBuilder;

internal static void FindPatternVariables(
Binder binder,
Expand All @@ -27,12 +27,12 @@ internal class PatternVariableFinder : CSharpSyntaxWalker

var finder = s_poolInstance.Allocate();
finder._binder = binder;
finder._declarationPatterns = builder;
finder._localsBuilder = builder;

finder.Visit(node);

finder._binder = null;
finder._declarationPatterns = null;
finder._localsBuilder = null;
s_poolInstance.Free(finder);
}

Expand All @@ -48,15 +48,15 @@ internal class PatternVariableFinder : CSharpSyntaxWalker

var finder = s_poolInstance.Allocate();
finder._binder = binder;
finder._declarationPatterns = builder;
finder._localsBuilder = builder;

foreach (var n in nodes)
{
finder.Visit(n);
}

finder._binder = null;
finder._declarationPatterns = null;
finder._localsBuilder = null;
s_poolInstance.Free(finder);
}

Expand Down Expand Up @@ -101,7 +101,7 @@ public override void VisitLockStatement(LockStatementSyntax node)

public override void VisitDeclarationPattern(DeclarationPatternSyntax node)
{
_declarationPatterns.Add(SourceLocalSymbol.MakeLocal(_binder.ContainingMemberOrLambda, _binder, RefKind.None, node.Type, node.Identifier, LocalDeclarationKind.PatternVariable));
_localsBuilder.Add(SourceLocalSymbol.MakeLocal(_binder.ContainingMemberOrLambda, _binder, RefKind.None, node.Type, node.Identifier, LocalDeclarationKind.PatternVariable));
base.VisitDeclarationPattern(node);
}
public override void VisitParenthesizedLambdaExpression(ParenthesizedLambdaExpressionSyntax node) { }
Expand Down Expand Up @@ -157,6 +157,17 @@ public override void VisitBinaryExpression(BinaryExpressionSyntax node)
operands.Free();
}

public override void VisitArgument(ArgumentSyntax node)
{
if (node.Expression != null)
{
base.VisitArgument(node);
return;
}

_localsBuilder.Add(SourceLocalSymbol.MakeLocal(_binder.ContainingMemberOrLambda, _binder, RefKind.None, node.Type, node.Identifier, LocalDeclarationKind.RegularVariable));
}

#region pool
private static readonly ObjectPool<PatternVariableFinder> s_poolInstance = CreatePool();

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@
<Compile Include="Symbols\Wrapped\WrappedPropertySymbol.cs" />
<Compile Include="Symbols\Wrapped\WrappedTypeParameterSymbol.cs" />
<Compile Include="Syntax\AliasedQualifiedNameSyntax.cs" />
<Compile Include="Syntax\ArgumentSyntax.cs" />
<Compile Include="Syntax\ArrayRankSpecifierSyntax.cs" />
<Compile Include="Syntax\AttributeSyntax.cs" />
<Compile Include="Syntax\AttributeTargetSpecifierSyntax.cs" />
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,15 @@ public static ISymbol GetDeclaredSymbol(this SemanticModel semanticModel, Variab
return csmodel?.GetDeclaredSymbol(declarationSyntax, cancellationToken);
}

/// <summary>
/// Given an argument syntax, get symbol for out var that it declares, if any.
/// </summary>
public static ISymbol GetDeclaredSymbol(this SemanticModel semanticModel, ArgumentSyntax declarationSyntax, CancellationToken cancellationToken = default(CancellationToken))
{
var csmodel = semanticModel as CSharpSemanticModel;
return csmodel?.GetDeclaredSymbol(declarationSyntax, cancellationToken);
}

/// <summary>
/// Given a declaration pattern syntax, get the corresponding symbol.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/CSharpParseOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ internal bool IsFeatureEnabled(MessageID feature)
case MessageID.IDS_FeaturePatternMatching:
case MessageID.IDS_FeatureTuples:
case MessageID.IDS_FeatureReplace:
case MessageID.IDS_FeatureOutVar:
// in "demo" mode enable proposed new C# 7 language features.
if (PreprocessorSymbols.Contains("__DEMO__"))
{
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -4872,4 +4872,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeatureTuples" xml:space="preserve">
<value>tuples</value>
</data>
<data name="IDS_FeatureOutVar" xml:space="preserve">
<value>out var</value>
</data>
</root>
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2657,6 +2657,14 @@ internal Conversion ClassifyConversionForCast(int position, ExpressionSyntax exp
/// <returns>The symbol that was declared.</returns>
public abstract ISymbol GetDeclaredSymbol(VariableDeclaratorSyntax declarationSyntax, CancellationToken cancellationToken = default(CancellationToken));

/// <summary>
/// Given an argument syntax, get symbol for an out var that it declares, if any.
/// </summary>
/// <param name="declarationSyntax">The syntax node that declares a variable.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The symbol that was declared.</returns>
public abstract ISymbol GetDeclaredSymbol(ArgumentSyntax declarationSyntax, CancellationToken cancellationToken = default(CancellationToken));

/// <summary>
/// Given a declaration pattern syntax, get the corresponding symbol.
/// </summary>
Expand Down Expand Up @@ -4551,6 +4559,8 @@ protected sealed override ISymbol GetDeclaredSymbolCore(SyntaxNode declaration,
return this.GetDeclaredSymbol((AnonymousObjectMemberDeclaratorSyntax)node, cancellationToken);
case SyntaxKind.VariableDeclarator:
return this.GetDeclaredSymbol((VariableDeclaratorSyntax)node, cancellationToken);
case SyntaxKind.Argument:
return this.GetDeclaredSymbol((ArgumentSyntax)node, cancellationToken);
case SyntaxKind.DeclarationPattern:
return this.GetDeclaredSymbol((DeclarationPatternSyntax)node, cancellationToken);
case SyntaxKind.NamespaceDeclaration:
Expand Down
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,18 @@ private LocalSymbol GetDeclaredLocal(CSharpSyntaxNode declarationSyntax, SyntaxT
return null;
}

public override ISymbol GetDeclaredSymbol(ArgumentSyntax declarationSyntax, CancellationToken cancellationToken = default(CancellationToken))
{
CheckSyntaxNode(declarationSyntax);

if (declarationSyntax.Expression != null)
{
return null;
}

return GetDeclaredLocal(declarationSyntax, declarationSyntax.Identifier);
}

public override ISymbol GetDeclaredSymbol(DeclarationPatternSyntax declarationSyntax, CancellationToken cancellationToken = default(CancellationToken))
{
CheckSyntaxNode(declarationSyntax);
Expand Down
Loading

0 comments on commit 68aca6f

Please sign in to comment.