Merge pull request #11493 from AlekseyTs/OutVar_02

Added support for implicitly-typed out variables.

docs/features/outvar.md

@@ -10,23 +10,35 @@ argument_value
     ;
 ```
 
-A variable declared this way is called an *out variable*. An *out variable* is read-only and scoped to the enclosing statement. More specifically, the scope will be the same as for a *pattern-variable* introduced via pattern-matching.
-
-> **Note**: We may treat *out variables* as *pattern variables* in the semantic model.
-
+A variable declared this way is called an *out variable*. 
 You may use the contextual keyword `var` for the variable's type.
+The scope will be the same as for a *pattern-variable* introduced via pattern-matching.
+Out variables are disallowed within constructor initializers.
 
-> **Open Issue**: The specification for overload resolution needs to be modified to account for the inference of the type of an *out variable*s declared with `var`.
+According to Language Specification (section 7.6.7 Element access)
+The argument-list of an element-access is not allowed to contain ref or out arguments.
+However, due to backward compatibility, compiler overlooks this restriction during parsing
+and even ignores out/ref modifiers in element access during binding.
+We will enforce that language rule for out variables declarations at the syntax level.
 
-An *out variable* may not be referenced before the close parenthesis of the invocation in which it is defined:
+Within the scope of a local variable introduced by a local-variable-declaration, 
+it is a compile-time error to refer to that local variable in a textual position 
+that precedes its declaration. 
 
-```cs
-    M(out x, x = 3); // error
-```
+It is also an error to reference implicitly-typed (§8.5.1) out variable in the same argument list that immediately 
+contains its declaration. Technically, we could support some restricted scenarios (when it is used as another 
+argument in the same list), but their utility is very questionable. Otherwise, we are looking into getting into 
+a cycle trying to infer its type. 
+
+For the purposes of overload resolution (see sections 7.5.3.2 Better function member and 7.5.3.3 Better conversion from expression),
+neither conversion is considered better when corresponding argument is an implicitly-typed out variable declaration.
+Once overload resolution succeeds, the type of implicitly-typed out variable is set to be equal to the type of the 
+corresponding parameter in the signature of the best candidate.
 
 > **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.
@@ -51,15 +63,5 @@ Added new API
 ```
 
 
-**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.
+**Open issues and TODOs:**
+    Tracked at https://github.com/dotnet/roslyn/issues/11566.

src/Compilers/CSharp/Portable/Binder/Binder.cs

@@ -168,6 +168,18 @@ internal virtual ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsFo
             return this.Next.GetDeclaredLocalFunctionsForScope(scopeDesignator);
         }
 
+        /// <summary>
+        /// If this binder owns a scope for locals, return syntax node that is used
+        /// as the scope designator. Otherwise, null.
+        /// </summary>
+        internal virtual SyntaxNode ScopeDesignator
+        {
+            get
+            {
+                return null;
+            }
+        }
+
         internal virtual bool IsLocalFunctionsScopeBinder
         {
             get

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

@@ -1369,12 +1369,43 @@ private BoundExpression BindNonMethod(SimpleNameSyntax node, Symbol symbol, Diag
                         }
                         else
                         {
+                            SourceLocalSymbol sourceLocal;
+                            type = null;
+
+                            if (node.SyntaxTree == localSymbolLocation.SourceTree && (object)(sourceLocal = localSymbol as SourceLocalSymbol) != null &&
+                                sourceLocal.IdentifierToken.Parent?.Kind() == SyntaxKind.Argument)
+                            {
+                                var argument = (ArgumentSyntax)sourceLocal.IdentifierToken.Parent;
+
+                                if (argument.Identifier == sourceLocal.IdentifierToken &&
+                                    argument.Type.IsVar)
+                                {
+                                    // We are referring to an out variable which might need type inference.
+                                    // If it is in fact needs inference, it is illegal to reference it in the same argument list that immediately 
+                                    // contains its declaration. 
+                                    // Technically, we could support some restricted scenarios (when it is used as another argument in the same list), 
+                                    // but their utility is very questionable. Otherwise, we are looking into getting into a cycle trying to infer its type. 
+                                    if (node.SpanStart < ((ArgumentListSyntax)argument.Parent).CloseParenToken.SpanStart && 
+                                        sourceLocal.IsVar) // This check involves trying to bind 'var' as a real type, so keeping it last for performance.
+                                    {
+                                        Error(diagnostics, ErrorCode.ERR_ImplicitlyTypedOutVariableUsedInTheSameArgumentList, node, node);
+
+                                        // Treat this case as variable used before declaration, we might be able to infer type of the variable anyway and SemanticModel 
+                                        // will be able to return non-error type information for this node. 
+                                        type = new ExtendedErrorTypeSymbol(this.Compilation, name: "var", arity: 0, errorInfo: null, variableUsedBeforeDeclaration: true);
+                                    }
+                                }
+                            }
+
+                            if ((object)type == null)
+                            {
+                                type = localSymbol.Type;
+                            }
+
                             if (IsBadLocalOrParameterCapture(localSymbol, localSymbol.RefKind))
                             {
                                 Error(diagnostics, ErrorCode.ERR_AnonDelegateCantUseLocal, node, localSymbol);
                             }
-
-                            type = localSymbol.Type;
                         }
 
                         return new BoundLocal(node, localSymbol, constantValueOpt, type, hasErrors: isError);
@@ -2121,10 +2152,21 @@ private BoundExpression BindArgumentValue(DiagnosticBag diagnostics, ArgumentSyn
                 this.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics);
             }
 
+            if (this.InConstructorInitializer)
+            {
+                Error(diagnostics, ErrorCode.ERR_OutVarInConstructorInitializer, argumentSyntax.Identifier);
+            }
+
             if (isVar)
             {
-                // PROTOTYPE(outvar):
-                throw new NotImplementedException();
+                return new OutVarLocalPendingInference(argumentSyntax, localSymbol);
+            }
+
+            if (this.ContainingMemberOrLambda.Kind == SymbolKind.Method
+                && ((MethodSymbol)this.ContainingMemberOrLambda).IsAsync
+                && declType.IsRestrictedType())
+            {
+                Error(diagnostics, ErrorCode.ERR_BadSpecialByRefLocal, argumentSyntax.Type, declType);
             }
 
             return new BoundLocal(argumentSyntax, localSymbol, constantValueOpt: null, type: declType);
@@ -2280,6 +2322,21 @@ private BoundExpression BindArgumentExpression(DiagnosticBag diagnostics, Expres
 
                     arguments[arg] = CreateConversion(argument.Syntax, argument, kind, false, type, diagnostics);
                 }
+                else if (argument.Kind == BoundKind.OutVarLocalPendingInference)
+                {
+                    TypeSymbol parameterType = GetCorrespondingParameterType(ref result, parameters, arg);
+                    bool hasErrors = false;
+
+                    if (this.ContainingMemberOrLambda.Kind == SymbolKind.Method
+                        && ((MethodSymbol)this.ContainingMemberOrLambda).IsAsync
+                        && parameterType.IsRestrictedType())
+                    {
+                        Error(diagnostics, ErrorCode.ERR_BadSpecialByRefLocal, ((ArgumentSyntax)argument.Syntax).Type, parameterType);
+                        hasErrors = true;
+                    }
+
+                    arguments[arg] = ((OutVarLocalPendingInference)argument).SetInferredType(parameterType, success: !hasErrors);
+                }
             }
         }
 

src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs

@@ -113,6 +113,7 @@ private static ImmutableArray<MethodSymbol> GetOriginalMethods(OverloadResolutio
             boundExpression.WasCompilerGenerated = true;
 
             var analyzedArguments = AnalyzedArguments.GetInstance();
+            Debug.Assert(!args.Any(e => e.Kind == BoundKind.OutVarLocalPendingInference));
             analyzedArguments.Arguments.AddRange(args);
             BoundExpression result = BindInvocationExpression(
                 node, node, methodName, boundExpression, analyzedArguments, diagnostics, queryClause,
@@ -329,6 +330,30 @@ private BoundExpression BindArgListOperator(InvocationExpressionSyntax node, Dia
 
         private ImmutableArray<BoundExpression> BuildArgumentsForDynamicInvocation(AnalyzedArguments arguments, DiagnosticBag diagnostics)
         {
+            for (int i = 0; i < arguments.Arguments.Count; i++)
+            {
+                if (arguments.Arguments[i].Kind == BoundKind.OutVarLocalPendingInference)
+                {
+                    var builder = ArrayBuilder<BoundExpression>.GetInstance(arguments.Arguments.Count);
+                    builder.AddRange(arguments.Arguments);
+
+                    do
+                    {
+                        BoundExpression argument = builder[i];
+
+                        if (argument.Kind == BoundKind.OutVarLocalPendingInference)
+                        {
+                            builder[i] = ((OutVarLocalPendingInference)argument).FailInference(this, diagnostics);
+                        }
+
+                        i++;
+                    }
+                    while (i < builder.Count);
+
+                    return builder.ToImmutableAndFree();
+                }
+            }
+
             return arguments.Arguments.ToImmutable();
         }
 
@@ -1100,7 +1125,8 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
             {
                 BoundKind argumentKind = oldArguments[i].Kind;
 
-                if (argumentKind == BoundKind.UnboundLambda && i < parameterCount)
+                if (argumentKind == BoundKind.OutVarLocalPendingInference || 
+                    (argumentKind == BoundKind.UnboundLambda && i < parameterCount))
                 {
                     ArrayBuilder<BoundExpression> newArguments = ArrayBuilder<BoundExpression>.GetInstance(argumentCount);
                     newArguments.AddRange(oldArguments);
@@ -1120,8 +1146,16 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
                                         newArguments[i] = ((UnboundLambda)oldArgument).Bind(parameterType);
                                     }
                                     break;
+
+                                case BoundKind.OutVarLocalPendingInference:
+                                    newArguments[i] = ((OutVarLocalPendingInference)oldArgument).SetInferredType(parameters[i].Type, success: true);
+                                    break;
                             }
                         }
+                        else if (oldArgument.Kind == BoundKind.OutVarLocalPendingInference)
+                        {
+                            newArguments[i] = ((OutVarLocalPendingInference)oldArgument).FailInference(this, null);
+                        }
 
                         i++;
                     }

src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs

@@ -1859,6 +1859,14 @@ private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind
                 case BoundKind.PropertyGroup:
                     expr = BindIndexedPropertyAccess((BoundPropertyGroup)expr, mustHaveAllOptionalParameters: false, diagnostics: diagnostics);
                     break;
+
+                case BoundKind.Local:
+                    Debug.Assert(expr.Syntax.Kind() != SyntaxKind.Argument || valueKind == BindValueKind.RefOrOut);
+                    break;
+
+                case BoundKind.OutVarLocalPendingInference:
+                    Debug.Assert(valueKind == BindValueKind.RefOrOut);
+                    return expr;
             }
 
             bool hasResolutionErrors = false;

src/Compilers/CSharp/Portable/Binder/BlockBinder.cs

@@ -85,6 +85,19 @@ private bool IsMatchingScopeDesignator(CSharpSyntaxNode scopeDesignator)
             return false;
         }
 
+        internal override SyntaxNode ScopeDesignator
+        {
+            get
+            {
+                if (_statements.Count == 1)
+                {
+                    return _statements.First();
+                }
+
+                return _statements.Node;
+            }
+        }
+
         internal override ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsForScope(CSharpSyntaxNode scopeDesignator)
         {
             if (IsMatchingScopeDesignator(scopeDesignator))

src/Compilers/CSharp/Portable/Binder/CatchClauseBinder.cs

@@ -51,5 +51,13 @@ internal override ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsF
         {
             throw ExceptionUtilities.Unreachable;
         }
+
+        internal override SyntaxNode ScopeDesignator
+        {
+            get
+            {
+                return _syntax;
+            }
+        }
     }
 }

src/Compilers/CSharp/Portable/Binder/FixedStatementBinder.cs

@@ -56,5 +56,13 @@ internal override ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsF
         {
             throw ExceptionUtilities.Unreachable;
         }
+
+        internal override SyntaxNode ScopeDesignator
+        {
+            get
+            {
+                return _syntax;
+            }
+        }
     }
 }

src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs

@@ -890,5 +890,13 @@ internal override ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsF
         {
             throw ExceptionUtilities.Unreachable;
         }
+
+        internal override SyntaxNode ScopeDesignator
+        {
+            get
+            {
+                return _syntax;
+            }
+        }
     }
 }

src/Compilers/CSharp/Portable/Binder/ForLoopBinder.cs

@@ -107,5 +107,13 @@ internal override ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsF
         {
             throw ExceptionUtilities.Unreachable;
         }
+
+        internal override SyntaxNode ScopeDesignator
+        {
+            get
+            {
+                return _syntax;
+            }
+        }
     }
 }

src/Compilers/CSharp/Portable/Binder/PatternVariableBinder.cs

@@ -11,8 +11,6 @@ namespace Microsoft.CodeAnalysis.CSharp
 {
     internal sealed class PatternVariableBinder : LocalScopeBinder
     {
-        public readonly CSharpSyntaxNode ScopeDesignator;
-
         internal PatternVariableBinder(CSharpSyntaxNode scopeDesignator, Binder next) : base(next)
         {
             this.ScopeDesignator = scopeDesignator;
@@ -21,7 +19,7 @@ internal PatternVariableBinder(CSharpSyntaxNode scopeDesignator, Binder next) :
         protected override ImmutableArray<LocalSymbol> BuildLocals()
         {
             var builder = ArrayBuilder<LocalSymbol>.GetInstance();
-            PatternVariableFinder.FindPatternVariables(this, builder, ScopeDesignator);
+            PatternVariableFinder.FindPatternVariables(this, builder, (CSharpSyntaxNode)ScopeDesignator);
             return builder.ToImmutableAndFree();
         }
 
@@ -39,5 +37,7 @@ internal override ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsF
         {
             throw ExceptionUtilities.Unreachable;
         }
+
+        internal override SyntaxNode ScopeDesignator { get; }
     }
 }

src/Compilers/CSharp/Portable/Binder/PatternVariableFinder.cs

@@ -165,6 +165,27 @@ public override void VisitArgument(ArgumentSyntax node)
                 return;
             }
 
+            var contextKind = node.Identifier.
+                                   Parent. // ArgumentSyntax
+                                   Parent. // ArgumentListSyntax
+                                   Parent. // invocation/constructor initializer
+                                   Kind();
+
+            switch (contextKind)
+            {
+                case SyntaxKind.InvocationExpression:
+                case SyntaxKind.ObjectCreationExpression:
+                case SyntaxKind.ThisConstructorInitializer:
+                case SyntaxKind.BaseConstructorInitializer:
+                    break;
+                default:
+
+                    // It looks like we are deling with a syntax tree that has a shape that could never be
+                    // produced by the LanguageParser, including all error conditions. 
+                    // Out Variable declarations can only appear in an argument list of the syntax nodes mentioned above.
+                    throw ExceptionUtilities.UnexpectedValue(contextKind);
+            }
+
             _localsBuilder.Add(SourceLocalSymbol.MakeLocal(_binder.ContainingMemberOrLambda, _binder, RefKind.None, node.Type, node.Identifier, LocalDeclarationKind.RegularVariable));
         }