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

Support multiple implicitly typed local variables in a single declaration. #5048

Closed
Closed
79 changes: 79 additions & 0 deletions docs/features/multiple-implicitly-typed-variables.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
### High Level Language Proposal:
Allow users to provide multiple declarators in an implicitly typed local ('var') declaration, as long as all of them have the same type.

Discussion:
Back when we added implicitly typed variables, we considered allowing multiple declarators in a single declaration. i.e.:

```c#
var x = 0, y = 0;
```

We decided against this because there was a lot of disagreement (including from the community) as to what the following should mean:

```c#
var x = 0, y = 1.0;
```

Opinions differed on if this would be equivalent to:

```c#
double x = 0, y = 1.0;
```

or

```c#
int x = 0; double y = 1.0;
```

Because of this large split in opinions, we decided to disallow this completely.

However, this is a very restrictive state to have ended up in. We can relax things a bit, without worrying about the above case being a problem. This is possible by allowing multiple variable declarations, but with the restriction that all of them have the same type. Because of this restriction, the above case would still be disallowed as 'x' would have the type 'int' and 'y' would have the type 'double'.

With this change, all existing allowed cases are still allowed. Some existing disallowed cases are still disallowed. But some very reasonable existing-disallowed cases are now allowed. These cases would have no confusion associated with them. So allowing them seems beneficial.


To accomplish this, we change the language specification like so:

> In the context of a local variable declaration, the identifier var acts as a contextual keyword (§2.4.3).When the local-variable-type is specified as var and no type named var is in scope, the declaration is an implicitly typed local variable declaration, whose type is inferred from the type of the associated initializer expression. Implicitly typed local variable declarations are subject to the following restrictions:
        ~~• The local-variable-declaration cannot include multiple local-variable-declarators.~~
        • The local-variable-declarator must include a local-variable-initializer.
        • The local-variable-initializer must be an expression.
        • The initializer expression must have a compile-time type.
        • The initializer expression cannot refer to the declared variable itself
        • + All initializer expressions must have the same compile-time type.

It is worth noting that the compile-time type of different declarators are not used to contextually type other declarators. For example the following are all illegal:

```c#
var x = "", y = null; // 'null' has no compile time type. This is not legal.
var x = (Func<int,int>)(a => a), y = b => b; // 'b => b' has no compile time type. This is not legal.
var x = (Action)() => {}, y = Console.WriteLine; // 'Console.WriteLine' has no compile time type. This is not legal.
```

However, despite this, it *is* possible for one variable declarator to influence another. For example:

```c#
var x = 0, y = x;
// This is legal. The compile time type of 'x' is Int32. The compile time type of
// 'y' is also Int32. Because 'x' is definitely assigned by the time it is read for 'y'.
```

Similar strange, but legal, forms are:

```c#
var a = 0, b = Foo(out a);
```


#### Grammar changes
None


#### Language Model changes
None (except for less errors than before).


#### Notes:
As before, implicitly typed class fields are not allowed. Nothing changes here with multiple vars.
However, implicitly typed script fields *are* allowed. As such, we allow multiple implicitly typed vars in script code.
105 changes: 74 additions & 31 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,78 @@ private BoundStatement BindDeclarationStatement(LocalDeclarationStatementSyntax
boundDeclarations[i++] = BindVariableDeclaration(kind, isVar, variableDeclaratorSyntax, typeSyntax, declType, alias, diagnostics);
}

return new BoundMultipleLocalDeclarations(node, boundDeclarations.AsImmutableOrNull());
var hasErrors = CheckMultipleVarDeclaration(node, variableList, isVar, boundDeclarations, diagnostics);

return new BoundMultipleLocalDeclarations(node, boundDeclarations.AsImmutableOrNull(), hasErrors);
}
}

// Returns 'true' if any errors are produced.
public bool CheckMultipleVarDeclaration(
CSharpSyntaxNode declarationNode,
SeparatedSyntaxList<VariableDeclaratorSyntax> declarators,
bool isVar,
BoundLocalDeclaration[] boundDeclarations,
DiagnosticBag diagnostics)
{
if (isVar && boundDeclarations.Length > 1 && !declarationNode.HasErrors)
{
foreach (var decl in boundDeclarations)
{
if (decl.HasAnyErrors)
{
// Don't bother checking if we ran into any errors already.
return false;
}
}

return CheckMultipleVarDeclaration(this.Compilation, declarationNode, declarators,
boundDeclarations.Select(d => d.LocalSymbol.Type).ToArray(), diagnostics);
}

return false;
}


// Returns 'true' if any errors are produced.
public static bool CheckMultipleVarDeclaration(
CSharpCompilation compilation,
CSharpSyntaxNode declarationNode,
SeparatedSyntaxList<VariableDeclaratorSyntax> declarators,
TypeSymbol[] types,
DiagnosticBag diagnostics)
{
Debug.Assert(declarators.Count > 1);
Debug.Assert(types.Length == declarators.Count);

// If this is a var declaration, then all types inferred must be identical.
var firstDeclarationType = types[0];

if (!firstDeclarationType.IsErrorType())
{
for (var i = 1; i < types.Length; i++)
{
var siblingDeclarationType = types[i];

if (!siblingDeclarationType.IsErrorType())
{
if (!firstDeclarationType.Equals(siblingDeclarationType))
{
// Report an error on the names of the locals that have different types.
var distinguisher = new SymbolDistinguisher(compilation, firstDeclarationType, siblingDeclarationType);
Error(diagnostics,
new CSDiagnosticInfo(ErrorCode.ERR_ImplicitlyTypedVariableMultipleDeclaratorSameType,
new object[] { distinguisher.First, distinguisher.Second },
ImmutableArray<Symbol>.Empty,
ImmutableArray.Create(declarators[i].Identifier.GetLocation())),
declarators[0].Identifier.GetLocation());
return true;
}
}
}
}

return false;
}

private TypeSymbol BindVariableType(CSharpSyntaxNode declarationNode, DiagnosticBag diagnostics, TypeSyntax typeSyntax, ref bool isConst, out bool isVar, out AliasSymbol alias)
Expand All @@ -509,27 +579,6 @@ private TypeSymbol BindVariableType(CSharpSyntaxNode declarationNode, Diagnostic
// Keep processing it as a non-const local.
isConst = false;
}

// In the dev10 compiler the error recovery semantics for the illegal case
// "var x = 10, y = 123.4;" are somewhat undesirable.
//
// First off, this is an error because a straw poll of language designers and
// users showed that there was no consensus on whether the above should mean
// "double x = 10, y = 123.4;", taking the best type available and substituting
// that for "var", or treating it as "var x = 10; var y = 123.4;" -- since there
// was no consensus we decided to simply make it illegal.
//
// In dev10 for error recovery in the IDE we do an odd thing -- we simply take
// the type of the first variable and use it. So that is "int x = 10, y = 123.4;".
//
// This seems less than ideal. In the error recovery scenario it probably makes
// more sense to treat that as "var x = 10; var y = 123.4;" and do each inference
// separately.

if (declarationNode.Kind() == SyntaxKind.LocalDeclarationStatement && ((LocalDeclarationStatementSyntax)declarationNode).Declaration.Variables.Count > 1 && !declarationNode.HasErrors)
{
Error(diagnostics, ErrorCode.ERR_ImplicitlyTypedVariableMultipleDeclarator, declarationNode);
}
}
else
{
Expand Down Expand Up @@ -2661,14 +2710,6 @@ internal BoundStatement BindForOrUsingOrFixedDeclarations(VariableDeclarationSyn
int count = variables.Count;
Debug.Assert(count > 0);

if (isVar && count > 1)
{
// There are a number of ways in which a var decl can be illegal, but in these
// cases we should report an error and then keep right on going with the inference.

Error(diagnostics, ErrorCode.ERR_ImplicitlyTypedVariableMultipleDeclarator, nodeOpt);
}

var declarationArray = new BoundLocalDeclaration[count];

for (int i = 0; i < count; i++)
Expand All @@ -2679,11 +2720,13 @@ internal BoundStatement BindForOrUsingOrFixedDeclarations(VariableDeclarationSyn
declarationArray[i] = declaration;
}

var hasErrors = CheckMultipleVarDeclaration(nodeOpt, variables, isVar, declarationArray, diagnostics);

declarations = declarationArray.AsImmutableOrNull();

return (count == 1) ?
(BoundStatement)declarations[0] :
new BoundMultipleLocalDeclarations(nodeOpt, declarations);
new BoundMultipleLocalDeclarations(nodeOpt, declarations, hasErrors);
}

internal BoundStatement BindStatementExpressionList(SeparatedSyntaxList<ExpressionSyntax> statements, DiagnosticBag diagnostics)
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 @@ -2075,6 +2075,9 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_ImplicitlyTypedVariableMultipleDeclarator" xml:space="preserve">
<value>Implicitly-typed variables cannot have multiple declarators</value>
</data>
<data name="ERR_ImplicitlyTypedVariableMultipleDeclaratorSameType" xml:space="preserve">
<value>Implicitly-typed variables must all have the same type. Types '{0}' and '{1}' are not the same</value>
</data>
<data name="ERR_ImplicitlyTypedVariableAssignedArrayInitializer" xml:space="preserve">
<value>Cannot initialize an implicitly-typed variable with an array initializer</value>
</data>
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,9 @@ internal enum ErrorCode
ERR_InvalidOutputName = 2041,
ERR_InvalidDebugInformationFormat = 2042,
ERR_LegacyObjectIdSyntax = 2043,
// unused 2044-2999
ERR_ImplicitlyTypedVariableMultipleDeclaratorSameType = 2044,
// unused 2045-2999

WRN_CLS_NoVarArgs = 3000,
WRN_CLS_BadArgType = 3001, // Requires SymbolDistinguisher.
WRN_CLS_BadReturnType = 3002,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1317,6 +1317,35 @@ protected void AfterMembersChecks(DiagnosticBag diagnostics)
CheckSequentialOnPartialType(diagnostics);
CheckForProtectedInStaticClass(diagnostics);
CheckForUnmatchedOperators(diagnostics);
CheckImplicitlyTypedFields(diagnostics);
}

private void CheckImplicitlyTypedFields(DiagnosticBag diagnostics)
{
// Only do this for script classes. In non-script-classes implicitly typed fields
// aren't even allowed.
if (this.IsScriptClass)
{
foreach (var member in this.GetMembers())
{
var field = member as SourceMemberFieldSymbol;
if (field != null && field.IsVar)
{
// When we hit the first declarator of an implicitly typed field, check if
// all declarators have the same type.
var fieldDeclaration = SourceMemberFieldSymbol.GetFieldDeclaration(field.VariableDeclaratorNode);
var declarators = fieldDeclaration.Declaration.Variables;

// Only bother if this actually has multiple declarators.
if (declarators.Count > 1 && declarators[0] == field.VariableDeclaratorNode)
{
var types = declarators.Select(d => GetMembers(d.Identifier.ValueText).OfType<SourceMemberFieldSymbol>().Single(
s => s.VariableDeclaratorNode == d).Type).ToArray();
Binder.CheckMultipleVarDeclaration(this.DeclaringCompilation, fieldDeclaration, declarators, types, diagnostics);
}
}
}
}
}

private void CheckMemberNamesDistinctFromType(DiagnosticBag diagnostics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public VariableDeclaratorSyntax VariableDeclaratorNode
}
}

private static BaseFieldDeclarationSyntax GetFieldDeclaration(CSharpSyntaxNode declarator)
internal static BaseFieldDeclarationSyntax GetFieldDeclaration(CSharpSyntaxNode declarator)
{
return (BaseFieldDeclarationSyntax)declarator.Parent.Parent;
}
Expand Down Expand Up @@ -167,6 +167,24 @@ private bool IsPointerFieldSyntactically()
return false;
}

public bool IsVar
{
get
{
var fieldDeclaration = GetFieldDeclaration(VariableDeclaratorNode);
if (fieldDeclaration.Declaration.Type.IsVar)
{
bool isVar;

var binder = GetDeclarationTypeBinder();
TypeSymbol declType = binder.BindType(fieldDeclaration.Declaration.Type, new DiagnosticBag(), out isVar);
return isVar;
}

return false;
}
}

internal sealed override TypeSymbol GetFieldType(ConsList<FieldSymbol> fieldsBeingBound)
{
Debug.Assert(fieldsBeingBound != null);
Expand Down Expand Up @@ -208,10 +226,7 @@ internal sealed override TypeSymbol GetFieldType(ConsList<FieldSymbol> fieldsBei
}
else
{
var binderFactory = compilation.GetBinderFactory(SyntaxTree);
var binder = binderFactory.GetBinder(typeSyntax);

binder = binder.WithContainingMemberOrLambda(this);
var binder = GetDeclarationTypeBinder();
if (!ContainingType.IsScriptClass)
{
type = binder.BindType(typeSyntax, diagnosticsForFirstDeclarator);
Expand Down Expand Up @@ -239,10 +254,6 @@ internal sealed override TypeSymbol GetFieldType(ConsList<FieldSymbol> fieldsBei
diagnostics.Add(ErrorCode.ERR_RecursivelyTypedVariable, this.ErrorLocation, this);
type = null;
}
else if (fieldSyntax.Declaration.Variables.Count > 1)
{
diagnosticsForFirstDeclarator.Add(ErrorCode.ERR_ImplicitlyTypedVariableMultipleDeclarator, typeSyntax.Location);
}
else
{
fieldsBeingBound = new ConsList<FieldSymbol>(this, fieldsBeingBound);
Expand Down Expand Up @@ -312,6 +323,14 @@ internal sealed override TypeSymbol GetFieldType(ConsList<FieldSymbol> fieldsBei
return _lazyType;
}

private Binder GetDeclarationTypeBinder()
{
var binder = this.DeclaringCompilation.GetBinderFactory(SyntaxTree).GetBinder(
GetFieldDeclaration(VariableDeclaratorNode).Declaration.Type);

return binder.WithContainingMemberOrLambda(this);
}

internal bool FieldTypeInferred(ConsList<FieldSymbol> fieldsBeingBound)
{
if (!ContainingType.IsScriptClass)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,14 @@ public void BindVarKeyword_MultipleDeclarators()
// Get the bind info for the text identified within the commented <bind> </bind> tags
var semanticInfo = GetBindInfoForTest(testSrc);

Assert.Equal("var", semanticInfo.Type.ToTestDisplayString());
Assert.Equal(TypeKind.Error, semanticInfo.Type.TypeKind);
Assert.Equal("var", semanticInfo.ConvertedType.ToTestDisplayString());
Assert.Equal(TypeKind.Error, semanticInfo.ConvertedType.TypeKind);
Assert.Equal("System.Int32", semanticInfo.Type.ToTestDisplayString());
Assert.Equal(TypeKind.Struct, semanticInfo.Type.TypeKind);
Assert.Equal("System.Int32", semanticInfo.ConvertedType.ToTestDisplayString());
Assert.Equal(TypeKind.Struct, semanticInfo.ConvertedType.TypeKind);
Assert.Equal(ConversionKind.Identity, semanticInfo.ImplicitConversion.Kind);

Assert.Null(semanticInfo.Symbol);
Assert.NotNull(semanticInfo.Symbol);
Assert.Equal("System.Int32", semanticInfo.Symbol.ToTestDisplayString());
Assert.Equal(CandidateReason.None, semanticInfo.CandidateReason);
Assert.Equal(0, semanticInfo.CandidateSymbols.Length);

Expand Down
Loading