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

Addressing blocking issues from test plan for inferred tuple names #19016

Merged
merged 10 commits into from
Apr 28, 2017
66 changes: 18 additions & 48 deletions src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -462,61 +462,53 @@ private static TypeSymbol MakeMergedTupleType(ArrayBuilder<DeconstructionVariabl
elementNames: default(ImmutableArray<string>),
compilation: compilation,
diagnostics: diagnostics,
shouldCheckConstraints: false);
shouldCheckConstraints: false,
errorPositions: default(ImmutableArray<bool>));
}

private BoundTupleLiteral DeconstructionVariablesAsTuple(CSharpSyntaxNode syntax, ArrayBuilder<DeconstructionVariable> variables, DiagnosticBag diagnostics, bool hasErrors)
{
bool inferNames = this.Compilation.LanguageVersion.InferTupleElementNames();
int count = variables.Count;
var valuesBuilder = ArrayBuilder<BoundExpression>.GetInstance(count);
var typesBuilder = ArrayBuilder<TypeSymbol>.GetInstance(count);
var locationsBuilder = ArrayBuilder<Location>.GetInstance(count);
var namesBuilder = inferNames ? ArrayBuilder<string>.GetInstance(count) : null;
var namesBuilder = ArrayBuilder<string>.GetInstance(count);
foreach (var variable in variables)
{
if (variable.HasNestedVariables)
{
var nestedTuple = DeconstructionVariablesAsTuple(variable.Syntax, variable.NestedVariables, diagnostics, hasErrors);
valuesBuilder.Add(nestedTuple);
typesBuilder.Add(nestedTuple.Type);
if (inferNames)
{
namesBuilder.Add(null);
}
namesBuilder.Add(null);
}
else
{
var single = variable.Single;
valuesBuilder.Add(single);
typesBuilder.Add(single.Type);
if (inferNames)
{
namesBuilder.Add(ExtractDeconstructResultElementName(single));
}
namesBuilder.Add(ExtractDeconstructResultElementName(single));
}
locationsBuilder.Add(variable.Syntax.Location);
}

ImmutableArray<string> tupleNames;
if (inferNames)
{
var uniqueFieldNames = PooledHashSet<string>.GetInstance();
RemoveDuplicateInferredTupleNames(namesBuilder, uniqueFieldNames);
uniqueFieldNames.Free();
tupleNames = namesBuilder.ToImmutableAndFree();
}
else
{
tupleNames = default(ImmutableArray<string>);
}
var uniqueFieldNames = PooledHashSet<string>.GetInstance();
RemoveDuplicateInferredTupleNames(namesBuilder, uniqueFieldNames);
uniqueFieldNames.Free();
Copy link
Member

Choose a reason for hiding this comment

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

Can uniqueFieldNames be moved to RemoveDuplicateInferredTupleNames as an implementation detail?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, no. Two of the usages use a fresh hashset (because they can't have explicit names in those scenarios, for instance in deconstruction), but one needs a hashset that already has explicit names (that's in BindTupleExpression).

ImmutableArray<string> tupleNames = namesBuilder.ToImmutableAndFree();

bool disallowInferredNames = this.Compilation.LanguageVersion.DisallowInferredTupleElementNames();
var inferredPositions = tupleNames.SelectAsArray(n => n != null);

var type = TupleTypeSymbol.Create(syntax.Location,
typesBuilder.ToImmutableAndFree(), locationsBuilder.ToImmutableAndFree(),
tupleNames, this.Compilation,
shouldCheckConstraints: !hasErrors, syntax: syntax, diagnostics: hasErrors ? null : diagnostics);
shouldCheckConstraints: !hasErrors,
errorPositions: disallowInferredNames ? inferredPositions : default(ImmutableArray<bool>),
syntax: syntax, diagnostics: hasErrors ? null : diagnostics);

var inferredPositions = inferNames ? tupleNames.SelectAsArray(n => n != null) : default(ImmutableArray<bool>);
// Always track the inferred positions in the bound node, so that conversions don't produce a warning
// for "dropped names" on tuple literal when the name was inferred.
return new BoundTupleLiteral(syntax, tupleNames, inferredPositions, arguments: valuesBuilder.ToImmutableAndFree(), type: type);
}

Expand All @@ -528,29 +520,7 @@ private static string ExtractDeconstructResultElementName(BoundExpression expres
return null;
}

SyntaxNode variableSyntax = expression.Syntax;
SyntaxToken nameToken;
if (variableSyntax.Kind() == SyntaxKind.SingleVariableDesignation)
{
nameToken = ((SingleVariableDesignationSyntax)variableSyntax).Identifier;
}
else if (variableSyntax.Kind() == SyntaxKind.DeclarationExpression)
{
var declaration = (DeclarationExpressionSyntax)variableSyntax;
nameToken = ((SingleVariableDesignationSyntax)declaration.Designation).Identifier;
}
else
{
nameToken = ((ExpressionSyntax)variableSyntax).ExtractAnonymousTypeMemberName();
}

string name = nameToken.ValueText;
if (name == null || TupleTypeSymbol.IsElementNameReserved(name) != -1)
{
return null;
}

return name;
return InferTupleElementName(expression.Syntax);
}

/// <summary>
Expand Down
99 changes: 35 additions & 64 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -742,23 +742,37 @@ private BoundExpression BindDeclarationVariables(TypeSymbol declType, VariableDe
case SyntaxKind.ParenthesizedVariableDesignation:
{
var tuple = (ParenthesizedVariableDesignationSyntax)node;
var builder = ArrayBuilder<BoundExpression>.GetInstance(tuple.Variables.Count);
int count = tuple.Variables.Count;
var builder = ArrayBuilder<BoundExpression>.GetInstance(count);
var namesBuilder = ArrayBuilder<string>.GetInstance(count);

foreach (var n in tuple.Variables)
{
builder.Add(BindDeclarationVariables(declType, n, n, diagnostics));
namesBuilder.Add(InferTupleElementName(n));
}
var subExpressions = builder.ToImmutableAndFree();

var uniqueFieldNames = PooledHashSet<string>.GetInstance();
RemoveDuplicateInferredTupleNames(namesBuilder, uniqueFieldNames);
uniqueFieldNames.Free();

var tupleNames = namesBuilder.ToImmutableAndFree();
var inferredPositions = tupleNames.SelectAsArray(n => n != null);
bool disallowInferredNames = this.Compilation.LanguageVersion.DisallowInferredTupleElementNames();

// We will not check constraints at this point as this code path
// is failure-only and the caller is expected to produce a diagnostic.
var tupleType = TupleTypeSymbol.Create(
null,
subExpressions.SelectAsArray(e => e.Type),
default(ImmutableArray<Location>),
default(ImmutableArray<string>),
tupleNames,
Compilation,
shouldCheckConstraints: false);
return new BoundTupleLiteral(syntax, default(ImmutableArray<string>), default(ImmutableArray<bool>), subExpressions, tupleType);
shouldCheckConstraints: false,
errorPositions: disallowInferredNames ? inferredPositions : default(ImmutableArray<bool>));

return new BoundTupleLiteral(syntax, default(ImmutableArray<string>), inferredPositions, subExpressions, tupleType);
}
default:
throw ExceptionUtilities.UnexpectedValue(node.Kind());
Expand Down Expand Up @@ -787,8 +801,7 @@ private BoundExpression BindTupleExpression(TupleExpressionSyntax node, Diagnost
var elementLocations = ArrayBuilder<Location>.GetInstance(arguments.Count);

// prepare names
var (elementNames, inferredPositions, hasErrors) = ExtractTupleElementNames(arguments, diagnostics,
withInference: this.Compilation.LanguageVersion.InferTupleElementNames());
var (elementNames, inferredPositions, hasErrors) = ExtractTupleElementNames(arguments, diagnostics);

// prepare types and locations
for (int i = 0; i < numElements; i++)
Expand Down Expand Up @@ -831,19 +844,24 @@ private BoundExpression BindTupleExpression(TupleExpressionSyntax node, Diagnost

if (hasNaturalType)
{
bool disallowInferredNames = this.Compilation.LanguageVersion.DisallowInferredTupleElementNames();

tupleTypeOpt = TupleTypeSymbol.Create(node.Location, elements, locations, elementNames,
this.Compilation, syntax: node, diagnostics: diagnostics, shouldCheckConstraints: true);
this.Compilation, syntax: node, diagnostics: diagnostics, shouldCheckConstraints: true,
errorPositions: disallowInferredNames ? inferredPositions : default(ImmutableArray<bool>));
}
else
{
TupleTypeSymbol.VerifyTupleTypePresent(elements.Length, node, this.Compilation, diagnostics);
}

// Always track the inferred positions in the bound node, so that conversions don't produce a warning
// for "dropped names" on tuple literal when the name was inferred.
return new BoundTupleLiteral(node, elementNames, inferredPositions, boundArguments.ToImmutableAndFree(), tupleTypeOpt, hasErrors);
}

private static (ImmutableArray<string> elementNamesArray, ImmutableArray<bool> inferredArray, bool hasErrors) ExtractTupleElementNames(
SeparatedSyntaxList<ArgumentSyntax> arguments, DiagnosticBag diagnostics, bool withInference)
SeparatedSyntaxList<ArgumentSyntax> arguments, DiagnosticBag diagnostics)
{
bool hasErrors = false;
int numElements = arguments.Count;
Expand All @@ -868,7 +886,7 @@ private static (ImmutableArray<string> elementNamesArray, ImmutableArray<bool> i
hasErrors = true;
}
}
else if (withInference)
else
{
inferredName = InferTupleElementName(argumentSyntax.Expression);
}
Expand Down Expand Up @@ -954,19 +972,17 @@ private static void RemoveDuplicateInferredTupleNames(ArrayBuilder<string> infer
toRemove.Free();
}

private static string InferTupleElementName(ExpressionSyntax element)
private static string InferTupleElementName(SyntaxNode syntax)
{
SyntaxToken nameToken = element.ExtractAnonymousTypeMemberName();
if (nameToken.Kind() == SyntaxKind.IdentifierToken)
string name = syntax.TryGetInferredMemberName();

// Reserved names are never candidates to be inferred names, at any position
if (name == null || TupleTypeSymbol.IsElementNameReserved(name) != -1)
{
string name = nameToken.ValueText;
// Reserved names are never candidates to be inferred names, at any position
if (TupleTypeSymbol.IsElementNameReserved(name) == -1)
{
return name;
}
return null;
}
return null;

return name;
}

private BoundExpression BindRefValue(RefValueExpressionSyntax node, DiagnosticBag diagnostics)
Expand Down Expand Up @@ -5540,58 +5556,13 @@ private void BindMemberAccessReportError(
{
Error(diagnostics, ErrorCode.ERR_NoSuchMemberOrExtensionNeedUsing, name, boundLeft.Type, plainName, "System");
}
else if (boundLeft.Type.IsTupleType && !Compilation.LanguageVersion.InferTupleElementNames()
&& IsMissingMemberAnInferredTupleName(boundLeft, plainName))
{
Error(diagnostics, ErrorCode.ERR_TupleInferredNamesNotAvailable, name, boundLeft.Type, plainName,
new CSharpRequiredLanguageVersion(LanguageVersion.CSharp7_1));
}
else
{
Error(diagnostics, ErrorCode.ERR_NoSuchMemberOrExtension, name, boundLeft.Type, plainName);
}
}
}

/// <summary>
/// When reporting a missing member error on a tuple type, determine whether that member
/// could be an element with an inferred name (had the language version been above 7.1).
/// </summary>
private static bool IsMissingMemberAnInferredTupleName(BoundExpression boundLeft, string plainName)
{
Debug.Assert(boundLeft.Type.IsTupleType);

var declarations = boundLeft.Type.DeclaringSyntaxReferences;
if (declarations.Length != 1)
{
return false;
}

var syntax = declarations[0].GetSyntax();
if (syntax.Kind() != SyntaxKind.TupleExpression)
{
return false;
}

var arguments = ((TupleExpressionSyntax)syntax).Arguments;
var (elementNames, inferredPositions, _) = ExtractTupleElementNames(arguments, diagnostics: null, withInference: true);

if (elementNames.IsDefault || inferredPositions.IsDefault)
{
return false;
}

for (int i = 0; i < arguments.Count; i++)
{
if (elementNames[i]?.Equals(plainName, StringComparison.Ordinal) == true && inferredPositions[i])
{
return true;
}
}

return false;
}

private bool WouldUsingSystemFindExtension(TypeSymbol receiver, string methodName)
{
// we have a special case to make the diagnostic for await expressions more clear for Windows:
Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,9 @@ private TypeSymbol BindTupleType(TupleTypeSyntax syntax, DiagnosticBag diagnosti
elementNames.ToImmutableAndFree(),
this.Compilation,
this.ShouldCheckConstraints,
syntax,
diagnostics);
errorPositions: default(ImmutableArray<bool>),
syntax: syntax,
diagnostics: diagnostics);
}

private static void CollectTupleFieldMemberName(string name, int elementIndex, int tupleSize, ref ArrayBuilder<string> elementNames)
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

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

2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5060,7 +5060,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<value>Compiler version: '{0}'. Language version: {1}.</value>
</data>
<data name="ERR_TupleInferredNamesNotAvailable" xml:space="preserve">
<value>Tuple type '{0}' does not have an explicitly named element '{1}'. Please use language version {2} or greater to access a unnamed element by its inferred name.</value>
<value>Tuple element name '{0}' is inferred. Please use language version {1} or greater to access an element by its inferred name.</value>
</data>
<data name="WRN_DefaultInSwitch" xml:space="preserve">
<value>Did you mean to use the default switch label (`default:`) rather than `case default:`? If you really mean to use the default literal, consider `case (default):` or another literal (`case 0:` or `case null:`) as appropriate.</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2733,7 +2733,8 @@ protected override INamedTypeSymbol CommonCreateTupleTypeSymbol(
elementLocations: elementLocations,
elementNames: elementNames,
compilation: this,
shouldCheckConstraints: false);
shouldCheckConstraints: false,
errorPositions: default(ImmutableArray<bool>));
}

protected override INamedTypeSymbol CommonCreateTupleTypeSymbol(
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ internal enum MessageID
IDS_FeatureExpressionBodiedDeOrConstructor = MessageBase + 12716,
IDS_ThrowExpression = MessageBase + 12717,
IDS_FeatureDefaultLiteral = MessageBase + 12718,
IDS_FeatureInferredTupleNames = MessageBase + 12719,
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -186,6 +187,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
{
// C# 7.1 features.
case MessageID.IDS_FeatureDefaultLiteral:
case MessageID.IDS_FeatureInferredTupleNames:
return LanguageVersion.CSharp7_1;

// C# 7 features.
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/LanguageVersion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,9 @@ public static LanguageVersion MapSpecifiedToEffectiveVersion(this LanguageVersio
}

/// <summary>Inference of tuple element names was added in C# 7.1</summary>
internal static bool InferTupleElementNames(this LanguageVersion self)
internal static bool DisallowInferredTupleElementNames(this LanguageVersion self)
{
return self >= LanguageVersion.CSharp7_1;
return self < MessageID.IDS_FeatureInferredTupleNames.RequiredVersion();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private BoundTupleLiteral ApplyDeconstructionConversion(ArrayBuilder<Binder.Deco

var tupleType = TupleTypeSymbol.Create(locationOpt: null, elementTypes: builder.SelectAsArray(e => e.Type),
elementLocations: default(ImmutableArray<Location>), elementNames: default(ImmutableArray<string>),
compilation: _compilation, shouldCheckConstraints: false);
compilation: _compilation, shouldCheckConstraints: false, errorPositions: default(ImmutableArray<bool>));

return new BoundTupleLiteral(right.Syntax, default(ImmutableArray<string>), default(ImmutableArray<bool>), builder.ToImmutableAndFree(), tupleType);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ static Microsoft.CodeAnalysis.CSharp.LanguageVersionFacts.MapSpecifiedToEffectiv
static Microsoft.CodeAnalysis.CSharp.LanguageVersionFacts.ToDisplayString(this Microsoft.CodeAnalysis.CSharp.LanguageVersion version) -> string
static Microsoft.CodeAnalysis.CSharp.LanguageVersionFacts.TryParse(this string version, out Microsoft.CodeAnalysis.CSharp.LanguageVersion result) -> bool
static Microsoft.CodeAnalysis.CSharp.SyntaxFacts.IsReservedTupleElementName(string elementName) -> bool
static Microsoft.CodeAnalysis.CSharp.SyntaxFacts.TryGetInferredMemberName(this Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax expression) -> string
static Microsoft.CodeAnalysis.CSharp.SyntaxFacts.TryGetInferredMemberName(this Microsoft.CodeAnalysis.SyntaxNode syntax) -> string
Loading