Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
268dea4
Implement lambda discard parameters
jcouv Sep 20, 2019
c8ba784
Remove duplicate tests
jcouv Sep 23, 2019
e8ed9dd
Factor logic to recognize underscore tokens
jcouv Sep 23, 2019
e9389c0
Align symbol display with other discards
jcouv Sep 23, 2019
43e255f
Distinguish discards in QuickInfo
jcouv Sep 23, 2019
6758a76
Add IsDiscard property instead of IDiscardSymbol
jcouv Sep 24, 2019
77dda47
Adjust IDE logic to use IsDiscard
jcouv Sep 24, 2019
570b3b0
Avoid large tuple return
jcouv Sep 24, 2019
63bbb77
Add test for ref/out discard parameters
jcouv Sep 24, 2019
161d3b8
Address PR feedback and test plan ideas
jcouv Oct 22, 2019
658fd21
Allow in expression trees
jcouv Oct 23, 2019
cadd8dc
Support general discard parameters
jcouv Oct 24, 2019
1dd6cd3
Add IDE tests
jcouv Oct 25, 2019
3c99305
Emit with unspeakable name
jcouv Oct 25, 2019
d30ee65
Remove unused error code
jcouv Oct 26, 2019
0d86ac4
Only support lambdas
jcouv Oct 28, 2019
f53b947
Fix tests
jcouv Oct 29, 2019
c2affd3
Address some PR feedback
jcouv Oct 30, 2019
0e71122
Address remaining feedback
jcouv Oct 31, 2019
84d2ae2
Address more feedback
jcouv Nov 1, 2019
4ed09e9
Merge branch 'master' into lambda-discards
jcouv Nov 1, 2019
29e4414
Fix conflict
jcouv Nov 1, 2019
a3462ef
Merge remote-tracking branch 'dotnet/master' into lambda-discards
jcouv Nov 5, 2019
01b013c
Update PublicAPI.Unshipped.txt
jcouv Nov 5, 2019
aa0dff8
Update PublicAPI.Unshipped.txt
jcouv Nov 5, 2019
9d910b8
Add ChangeSignature tests
jcouv Nov 12, 2019
554d19c
Add SymbolCompletion tests
jcouv Nov 12, 2019
07739c6
Add InlineRename test
jcouv Nov 12, 2019
8f95c6b
Typo
jcouv Nov 12, 2019
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public QueryUnboundLambdaState(Binder binder, RangeVariableMap rangeVariableMap,
}

public override string ParameterName(int index) { return _parameters[index].Name; }
public override bool ParameterIsDiscard(int index) { return false; }
public override bool HasNames { get { return true; } }
public override bool HasSignature { get { return true; } }
public override bool HasExplicitlyTypedParameterList { get { return false; } }
public override int ParameterCount { get { return _parameters.Length; } }
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1424,7 +1424,7 @@ private BoundExpression BindIdentifier(
/// </summary>
private static bool FallBackOnDiscard(IdentifierNameSyntax node, DiagnosticBag diagnostics)
{
if (node.Identifier.ContextualKind() != SyntaxKind.UnderscoreToken)
if (!node.Identifier.IsUnderscoreToken())
{
return false;
}
Expand All @@ -1441,7 +1441,7 @@ private static bool FallBackOnDiscard(IdentifierNameSyntax node, DiagnosticBag d

private static bool IsOutVarDiscardIdentifier(SimpleNameSyntax node)
{
Debug.Assert(node.Identifier.ContextualKind() == SyntaxKind.UnderscoreToken);
Debug.Assert(node.Identifier.IsUnderscoreToken());

CSharpSyntaxNode parent = node.Parent;
return (parent?.Kind() == SyntaxKind.Argument &&
Expand Down
57 changes: 50 additions & 7 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal partial class Binder
// If we have no modifiers then the modifiers array is null; if we have any modifiers
// then the modifiers array is non-null and not empty.

private (ImmutableArray<RefKind>, ImmutableArray<TypeWithAnnotations>, ImmutableArray<string>, bool) AnalyzeAnonymousFunction(
private UnboundLambda AnalyzeAnonymousFunction(
CSharpSyntaxNode syntax, DiagnosticBag diagnostics)
{
Debug.Assert(syntax != null);
Expand All @@ -43,6 +43,7 @@ internal partial class Binder
bool isAsync = false;

var namesBuilder = ArrayBuilder<string>.GetInstance();
ImmutableArray<bool> discardsOpt = default;
SeparatedSyntaxList<ParameterSyntax>? parameterSyntaxList = null;
bool hasSignature;

Expand Down Expand Up @@ -94,8 +95,14 @@ internal partial class Binder
// However, we still want to give errors on every bad type in the list, even if one
// is missing.

int underscoresCount = 0;
foreach (var p in parameterSyntaxList.Value)
{
if (p.Identifier.IsUnderscoreToken())
{
underscoresCount++;
}

foreach (var attributeList in p.AttributeLists)
{
Error(diagnostics, ErrorCode.ERR_AttributesNotAllowed, attributeList);
Expand Down Expand Up @@ -161,6 +168,8 @@ internal partial class Binder
refKindsBuilder.Add(refKind);
}

discardsOpt = computeDiscards(parameterSyntaxList.Value, underscoresCount);

if (hasExplicitlyTypedParameterList)
{
types = typesBuilder.ToImmutable();
Expand All @@ -182,7 +191,24 @@ internal partial class Binder

namesBuilder.Free();

return (refKinds, types, names, isAsync);
return new UnboundLambda(syntax, this, refKinds, types, names, discardsOpt, isAsync);

static ImmutableArray<bool> computeDiscards(SeparatedSyntaxList<ParameterSyntax> parameters, int underscoresCount)
{
if (underscoresCount <= 1)
{
return default;
}

// When there are two or more underscores, they are discards
var discardsBuilder = ArrayBuilder<bool>.GetInstance(parameters.Count);
foreach (var p in parameters)
{
discardsBuilder.Add(p.Identifier.IsUnderscoreToken());
}

return discardsBuilder.ToImmutableAndFree();
}
}

private void CheckParenthesizedLambdaParameters(
Expand Down Expand Up @@ -216,25 +242,27 @@ private UnboundLambda BindAnonymousFunction(CSharpSyntaxNode syntax, DiagnosticB
Debug.Assert(syntax != null);
Debug.Assert(syntax.IsAnonymousFunction());

var (refKinds, types, names, isAsync) = AnalyzeAnonymousFunction(syntax, diagnostics);
if (!types.IsDefault)
var lambda = AnalyzeAnonymousFunction(syntax, diagnostics);
var data = lambda.Data;
if (data.HasExplicitlyTypedParameterList)
{
foreach (var type in types)
for (int i = 0; i < lambda.ParameterCount; i++)
{
// UNDONE: Where do we report improper use of pointer types?
var type = lambda.Data.ParameterTypeWithAnnotations(i);
if (type.HasType && type.IsStatic)
{
Error(diagnostics, ErrorCode.ERR_ParameterIsStaticClass, syntax, type.Type);
}
}
}

var lambda = new UnboundLambda(syntax, this, refKinds, types, names, isAsync);
if (!names.IsDefault)
if (data.HasNames)
{
var binder = new LocalScopeBinder(this);
bool allowShadowingNames = binder.Compilation.IsFeatureEnabled(MessageID.IDS_FeatureNameShadowingInNestedFunctions);
var pNames = PooledHashSet<string>.GetInstance();
bool seenDiscard = false;

for (int i = 0; i < lambda.ParameterCount; i++)
{
Expand All @@ -245,6 +273,21 @@ private UnboundLambda BindAnonymousFunction(CSharpSyntaxNode syntax, DiagnosticB
continue;
}

if (lambda.ParameterIsDiscard(i))
{
if (seenDiscard)
{
// We only report the diagnostic on the second and subsequent underscores
MessageID.IDS_FeatureLambdaDiscardParameters.CheckFeatureAvailability(
diagnostics,
binder.Compilation,
lambda.ParameterLocation(i));
}

seenDiscard = true;
continue;

@AlekseyTs AlekseyTs Oct 29, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

continue; [](start = 24, length = 9)

It doesn't feel valid to continue for the single discard case. #Closed

@AlekseyTs AlekseyTs Oct 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

continue; [](start = 24, length = 9)

Consider adding a test that would verify that we don't get here for a regular parameter named _. I.e. we are still going to report a diagnostics from ValidateLambdaParameterNameConflictsInScope when it is appropriate. #Closed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added test DiscardParameters_SingleUnderscoreParameter


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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added test DiscardParameters_SingleUnderscoreParameter

I doesn't look like the test is covering the scenario. I would expect an error about parameter having a conflicting name with something in an outer scope. The test is asserting a conflict of something declared in a nested scope with a parameter instead. Therefore, doesn't test the code path.


In reply to: 341279707 [](ancestors = 341279707,340741002)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm adding another test, but this ValidateLambdaParameterNameConflictsInScope won't be reachable. allowShadowingNames is true as of C# 8.


In reply to: 341297823 [](ancestors = 341297823,341279707,340741002)

}

if (!pNames.Add(name))
{
// The parameter name '{0}' is a duplicate
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2752,7 +2752,7 @@ private BoundExpression BindIsOperator(BinaryExpressionSyntax node, DiagnosticBa
TypeWithAnnotations targetTypeWithAnnotations = BindType(node.Right, isTypeDiagnostics, out alias);
TypeSymbol targetType = targetTypeWithAnnotations.Type;

bool wasUnderscore = node.Right is IdentifierNameSyntax name && name.Identifier.ContextualKind() == SyntaxKind.UnderscoreToken;
bool wasUnderscore = node.Right is IdentifierNameSyntax name && name.Identifier.IsUnderscoreToken();
if (!wasUnderscore && targetType?.IsErrorType() == true && isTypeDiagnostics.HasAnyResolvedErrors() &&
((CSharpParseOptions)node.SyntaxTree.Options).IsFeatureEnabled(MessageID.IDS_FeaturePatternMatching))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ private BoundSwitchLabel BindSwitchSectionLabel(
void reportIfConstantNamedUnderscore(BoundPattern pattern, ExpressionSyntax expression)
{
if (!pattern.HasErrors &&
expression is IdentifierNameSyntax name && name.Identifier.ContextualKind() == SyntaxKind.UnderscoreToken)
expression is IdentifierNameSyntax name && name.Identifier.IsUnderscoreToken())
{
diagnostics.Add(ErrorCode.WRN_CaseConstantNamedUnderscore, expression.Location);
}
Expand Down
21 changes: 12 additions & 9 deletions src/Compilers/CSharp/Portable/Binder/WithLambdaParametersBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,25 @@ public WithLambdaParametersBinder(LambdaSymbol lambdaSymbol, Binder enclosing)
var parameters = lambdaSymbol.Parameters;
if (!parameters.IsDefaultOrEmpty)
{
RecordDefinitions(parameters);
recordDefinitions(parameters);
foreach (var parameter in lambdaSymbol.Parameters)
{
this.parameterMap.Add(parameter.Name, parameter);
if (!parameter.IsDiscard)
{
this.parameterMap.Add(parameter.Name, parameter);
}
}
}
}

private void RecordDefinitions(ImmutableArray<ParameterSymbol> definitions)
{
var declarationMap = _definitionMap ?? (_definitionMap = new SmallDictionary<string, ParameterSymbol>());
foreach (var s in definitions)
void recordDefinitions(ImmutableArray<ParameterSymbol> definitions)
{
if (!declarationMap.ContainsKey(s.Name))
var declarationMap = _definitionMap ??= new SmallDictionary<string, ParameterSymbol>();
foreach (var s in definitions)
{
declarationMap.Add(s.Name, s);
if (!s.IsDiscard && !declarationMap.ContainsKey(s.Name))
{
declarationMap.Add(s.Name, s);
}
}
}
}
Expand Down
18 changes: 16 additions & 2 deletions src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -327,13 +327,14 @@ public UnboundLambda(
ImmutableArray<RefKind> refKinds,
ImmutableArray<TypeWithAnnotations> types,
ImmutableArray<string> names,
ImmutableArray<bool> discardsOpt,
bool isAsync,
bool hasErrors = false)
: base(BoundKind.UnboundLambda, syntax, null, hasErrors || !types.IsDefault && types.Any(t => t.Type?.Kind == SymbolKind.ErrorType))
{
Debug.Assert(binder != null);
Debug.Assert(syntax.IsAnonymousFunction());
this.Data = new PlainUnboundLambdaState(this, binder, names, types, refKinds, isAsync);
this.Data = new PlainUnboundLambdaState(this, binder, names, discardsOpt, types, refKinds, isAsync);
}

private UnboundLambda(UnboundLambda other, Binder binder, NullableWalker.VariableState nullableState) :
Expand Down Expand Up @@ -376,6 +377,7 @@ public TypeWithAnnotations InferReturnType(ConversionsBase conversions, NamedTyp
public TypeSymbol ParameterType(int index) { return ParameterTypeWithAnnotations(index).Type; }
public Location ParameterLocation(int index) { return Data.ParameterLocation(index); }
public string ParameterName(int index) { return Data.ParameterName(index); }
public bool ParameterIsDiscard(int index) { return Data.ParameterIsDiscard(index); }
}

internal abstract class UnboundLambdaState
Expand Down Expand Up @@ -415,13 +417,15 @@ public void SetUnboundLambda(UnboundLambda unbound)

public abstract MessageID MessageID { get; }
public abstract string ParameterName(int index);
public abstract bool ParameterIsDiscard(int index);
public abstract bool HasSignature { get; }
public abstract bool HasExplicitlyTypedParameterList { get; }
public abstract int ParameterCount { get; }
public abstract bool IsAsync { get; }
public abstract bool HasNames { get; }

public abstract Location ParameterLocation(int index);
public abstract TypeWithAnnotations ParameterTypeWithAnnotations(int index);
//public abstract SyntaxToken ParameterIdentifier(int index);
public abstract RefKind RefKind(int index);
protected abstract BoundBlock BindLambdaBody(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, DiagnosticBag diagnostics);

Expand Down Expand Up @@ -1078,6 +1082,7 @@ private static int CanonicallyCompareDiagnostics(Diagnostic x, Diagnostic y)
internal class PlainUnboundLambdaState : UnboundLambdaState
{
private readonly ImmutableArray<string> _parameterNames;
private readonly ImmutableArray<bool> _parameterIsDiscardOpt;
private readonly ImmutableArray<TypeWithAnnotations> _parameterTypesWithAnnotations;
private readonly ImmutableArray<RefKind> _parameterRefKinds;
private readonly bool _isAsync;
Expand All @@ -1086,17 +1091,21 @@ internal PlainUnboundLambdaState(
UnboundLambda unboundLambda,
Binder binder,
ImmutableArray<string> parameterNames,
ImmutableArray<bool> parameterIsDiscardOpt,
ImmutableArray<TypeWithAnnotations> parameterTypesWithAnnotations,
ImmutableArray<RefKind> parameterRefKinds,
bool isAsync)
: base(binder, unboundLambda)
{
_parameterNames = parameterNames;
_parameterIsDiscardOpt = parameterIsDiscardOpt;
_parameterTypesWithAnnotations = parameterTypesWithAnnotations;
_parameterRefKinds = parameterRefKinds;
_isAsync = isAsync;
}

public override bool HasNames { get { return !_parameterNames.IsDefault; } }

public override bool HasSignature { get { return !_parameterNames.IsDefault; } }

public override bool HasExplicitlyTypedParameterList { get { return !_parameterTypesWithAnnotations.IsDefault; } }
Expand Down Expand Up @@ -1139,6 +1148,11 @@ public override string ParameterName(int index)
return _parameterNames[index];
}

public override bool ParameterIsDiscard(int index)
{
return _parameterIsDiscardOpt.IsDefault ? false : _parameterIsDiscardOpt[index];
}

public override RefKind RefKind(int index)
{
Debug.Assert(0 <= index && index < _parameterTypesWithAnnotations.Length);
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ internal static SyntaxKind ContextualKind(this SyntaxToken token)
return (object)token.Language == (object)LanguageNames.CSharp ? (SyntaxKind)token.RawContextualKind : SyntaxKind.None;
}

internal static bool IsUnderscoreToken(this SyntaxToken identifier)
{
return identifier.ContextualKind() == SyntaxKind.UnderscoreToken;
}

/// <summary>
/// Returns the index of the first node of a specified kind in the node list.
/// </summary>
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 @@ -5744,6 +5744,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeatureNameShadowingInNestedFunctions" xml:space="preserve">
<value>name shadowing in nested functions</value>
</data>
<data name="IDS_FeatureLambdaDiscardParameters" xml:space="preserve">
<value>lambda discard parameters</value>
</data>
<data name="ERR_BadDynamicAwaitForEach" xml:space="preserve">
<value>Cannot use a collection of dynamic type in an asynchronous foreach</value>
</data>
Expand Down
6 changes: 5 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ internal enum MessageID
IDS_FeatureNestedStackalloc = MessageBase + 12762,
IDS_FeatureSwitchExpression = MessageBase + 12763,
IDS_FeatureAsyncUsing = MessageBase + 12764,
IDS_FeatureLambdaDiscardParameters = MessageBase + 12765,
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -282,7 +283,6 @@ private static CSDiagnosticInfo GetDisabledFeatureDiagnosticInfo(MessageID featu
: new CSDiagnosticInfo(availableVersion.GetErrorCode(), feature.Localize(), new CSharpRequiredLanguageVersion(requiredVersion));
}


internal static LanguageVersion RequiredVersion(this MessageID feature)
{
Debug.Assert(RequiredFeature(feature) == null);
Expand All @@ -291,6 +291,10 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
// Checks are in the LanguageParser unless otherwise noted.
switch (feature)
{
// Preview features.
case MessageID.IDS_FeatureLambdaDiscardParameters: // semantic check
return LanguageVersion.Preview;

// C# 8.0 features.
case MessageID.IDS_FeatureAltInterpolatedVerbatimStrings:
case MessageID.IDS_FeatureCoalesceAssignmentExpression:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,14 @@ public override int Ordinal
}
}

public override bool IsDiscard
{
get
{
return false;
}
}

// might be Nil
internal ParameterHandle Handle
{
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ParameterSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ protected override sealed Symbol OriginalSymbolDefinition
/// </summary>
public abstract RefKind RefKind { get; }

/// <summary>
/// Returns true if the parameter is a discard parameter.
/// </summary>
public abstract bool IsDiscard { get; }

@AlekseyTs AlekseyTs Oct 29, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

public abstract bool IsDiscard { get; } [](start = 8, length = 39)

Consider adding an explicit implementation for IParameterSymbol.IsDiscard. This will make it easier to deal with a merge conflict with the change that splits ISymbols and Symbols for C#. #Closed


/// <summary>
/// Custom modifiers associated with the ref modifier, or an empty array if there are none.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ IParameterSymbol IParameterSymbol.OriginalDefinition

RefKind IParameterSymbol.RefKind => _underlying.RefKind;

bool IParameterSymbol.IsDiscard => _underlying.IsDiscard;

bool IParameterSymbol.IsParams => _underlying.IsParams;

bool IParameterSymbol.IsOptional => _underlying.IsOptional;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public SignatureOnlyParameterSymbol(

public override bool IsImplicitlyDeclared { get { return true; } }

public override bool IsDiscard { get { return false; } }

#region Not used by MethodSignatureComparer

internal override bool IsMetadataIn { get { throw ExceptionUtilities.Unreachable; } }
Expand Down
Loading