Skip to content

Commit

Permalink
Revert "Scoped implicitly-typed lambda parameters (#64680)" (#64986)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv authored Oct 27, 2022
1 parent 208c7da commit c50d5e8
Show file tree
Hide file tree
Showing 9 changed files with 184 additions and 130 deletions.
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,7 @@ Symbol remapLambda(BoundLambda boundLambda, LambdaSymbol lambda)
if (updatedDelegateType is null)
{
Debug.Assert(updatedContaining is object);
var parameterEffectiveScopes = lambda.GetParameterEffectiveScopes();
updatedLambda = boundLambda.CreateLambdaSymbol(updatedContaining, lambda.ReturnTypeWithAnnotations, lambda.ParameterTypesWithAnnotations, lambda.ParameterRefKinds, parameterEffectiveScopes, lambda.RefKind);
updatedLambda = boundLambda.CreateLambdaSymbol(updatedContaining, lambda.ReturnTypeWithAnnotations, lambda.ParameterTypesWithAnnotations, lambda.ParameterRefKinds, lambda.RefKind);
}
else
{
Expand Down
77 changes: 24 additions & 53 deletions src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
View file

Large diffs are not rendered by default.

12 changes: 0 additions & 12 deletions src/Compilers/CSharp/Portable/Symbols/MethodSymbolExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,17 +225,5 @@ internal static bool IsValidUnscopedRefAttributeTarget(this MethodSymbol method)
!method.IsConstructor() &&
!method.IsInitOnly;
}

#nullable enable
internal static ImmutableArray<DeclarationScope> GetParameterEffectiveScopes(this MethodSymbol? method)
{
if (method is null)
return default;

if (method.Parameters.All(p => p.EffectiveScope == DeclarationScope.Unscoped))
return default;

return method.Parameters.SelectAsArray(p => p.EffectiveScope);
}
}
}
11 changes: 2 additions & 9 deletions src/Compilers/CSharp/Portable/Symbols/Source/LambdaParameterSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Roslyn.Utilities;

Expand All @@ -12,31 +11,25 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
internal sealed class LambdaParameterSymbol : SourceComplexParameterSymbolBase
{
private readonly SyntaxList<AttributeListSyntax> _attributeLists;
private readonly DeclarationScope? _effectiveScope;

public LambdaParameterSymbol(
LambdaSymbol owner,
SyntaxList<AttributeListSyntax> attributeLists,
TypeWithAnnotations parameterType,
int ordinal,
RefKind refKind,
DeclarationScope? declaredScope,
DeclarationScope? effectiveScope,
DeclarationScope scope,
string name,
bool isDiscard,
ImmutableArray<Location> locations)
: base(owner, ordinal, parameterType, refKind, name, locations, syntaxRef: null, isParams: false, isExtensionMethodThis: false, scope: declaredScope)
: base(owner, ordinal, parameterType, refKind, name, locations, syntaxRef: null, isParams: false, isExtensionMethodThis: false, scope)
{
Debug.Assert(declaredScope.HasValue != effectiveScope.HasValue);
_attributeLists = attributeLists;
_effectiveScope = effectiveScope;
IsDiscard = isDiscard;
}

public override bool IsDiscard { get; }

internal override DeclarationScope EffectiveScope => _effectiveScope ?? base.EffectiveScope;

internal override bool IsMetadataOptional
{
get { return false; }
Expand Down
34 changes: 10 additions & 24 deletions src/Compilers/CSharp/Portable/Symbols/Source/LambdaSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
Expand Down Expand Up @@ -43,14 +42,12 @@ public LambdaSymbol(
UnboundLambda unboundLambda,
ImmutableArray<TypeWithAnnotations> parameterTypes,
ImmutableArray<RefKind> parameterRefKinds,
ImmutableArray<DeclarationScope> parameterEffectiveScopes,
RefKind refKind,
TypeWithAnnotations returnType) :
base(unboundLambda.Syntax.GetReference())
{
Debug.Assert(syntaxReferenceOpt is not null);
Debug.Assert(containingSymbol.DeclaringCompilation == compilation);
Debug.Assert(parameterEffectiveScopes.IsDefault || parameterTypes.Length == parameterEffectiveScopes.Length);

_binder = binder;
_containingSymbol = containingSymbol;
Expand All @@ -65,7 +62,7 @@ public LambdaSymbol(
_isAsync = unboundLambda.IsAsync;
_isStatic = unboundLambda.IsStatic;
// No point in making this lazy. We are always going to need these soon after creation of the symbol.
_parameters = MakeParameters(compilation, unboundLambda, parameterTypes, parameterRefKinds, parameterEffectiveScopes);
_parameters = MakeParameters(compilation, unboundLambda, parameterTypes, parameterRefKinds);
_declarationDiagnostics = new BindingDiagnosticBag();
}

Expand Down Expand Up @@ -307,11 +304,9 @@ private ImmutableArray<ParameterSymbol> MakeParameters(
CSharpCompilation compilation,
UnboundLambda unboundLambda,
ImmutableArray<TypeWithAnnotations> parameterTypes,
ImmutableArray<RefKind> parameterRefKinds,
ImmutableArray<DeclarationScope> parameterEffectiveScopes)
ImmutableArray<RefKind> parameterRefKinds)
{
Debug.Assert(parameterTypes.Length == parameterRefKinds.Length);
Debug.Assert(parameterEffectiveScopes.IsDefault || parameterTypes.Length == parameterEffectiveScopes.Length);

if (!unboundLambda.HasSignature || unboundLambda.ParameterCount == 0)
{
Expand All @@ -322,9 +317,8 @@ private ImmutableArray<ParameterSymbol> MakeParameters(
type,
ordinal,
arg.refKinds[ordinal],
GeneratedNames.LambdaCopyParameterName(ordinal), // Make sure nothing binds to this.
getScope(arg.parameterEffectiveScopes, ordinal)),
(owner: this, refKinds: parameterRefKinds, parameterEffectiveScopes));
GeneratedNames.LambdaCopyParameterName(ordinal)), // Make sure nothing binds to this.
(owner: this, refKinds: parameterRefKinds));
}

var builder = ArrayBuilder<ParameterSymbol>.GetInstance(unboundLambda.ParameterCount);
Expand All @@ -341,46 +335,38 @@ private ImmutableArray<ParameterSymbol> MakeParameters(

TypeWithAnnotations type;
RefKind refKind;
DeclarationScope? declaredScope;
DeclarationScope? effectiveScope;
DeclarationScope scope;
if (hasExplicitlyTypedParameterList)
{
type = unboundLambda.ParameterTypeWithAnnotations(p);
refKind = unboundLambda.RefKind(p);
declaredScope = unboundLambda.DeclaredScope(p);
effectiveScope = null;
scope = unboundLambda.DeclaredScope(p);
}
else if (p < numDelegateParameters)
{
type = parameterTypes[p];
refKind = parameterRefKinds[p];
declaredScope = null;
effectiveScope = getScope(parameterEffectiveScopes, p);
scope = DeclarationScope.Unscoped;
}
else
{
type = TypeWithAnnotations.Create(new ExtendedErrorTypeSymbol(compilation, name: string.Empty, arity: 0, errorInfo: null));
refKind = RefKind.None;
declaredScope = DeclarationScope.Unscoped;
effectiveScope = null;
scope = DeclarationScope.Unscoped;
}

var attributeLists = unboundLambda.ParameterAttributes(p);
var name = unboundLambda.ParameterName(p);
var location = unboundLambda.ParameterLocation(p);
var locations = location == null ? ImmutableArray<Location>.Empty : ImmutableArray.Create<Location>(location);

var parameter = new LambdaParameterSymbol(owner: this, attributeLists, type, ordinal: p, refKind, declaredScope, effectiveScope, name, unboundLambda.ParameterIsDiscard(p), locations);
var parameter = new LambdaParameterSymbol(owner: this, attributeLists, type, ordinal: p, refKind, scope, name, unboundLambda.ParameterIsDiscard(p), locations);
builder.Add(parameter);
}

var result = builder.ToImmutableAndFree();
return result;

static DeclarationScope getScope(ImmutableArray<DeclarationScope> scopes, int ordinal)
{
return scopes.IsDefault ? DeclarationScope.Unscoped : scopes[ordinal];
}
return result;
}

public sealed override bool Equals(Symbol symbol, TypeCompareKind compareKind)
Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ protected SourceComplexParameterSymbolBase(
SyntaxReference syntaxRef,
bool isParams,
bool isExtensionMethodThis,
DeclarationScope? scope)
DeclarationScope scope)
: base(owner, parameterType, ordinal, refKind, scope, name, locations)
{
Debug.Assert((syntaxRef == null) || (syntaxRef.GetSyntax().IsKind(SyntaxKind.Parameter)));
Expand Down Expand Up @@ -198,7 +198,7 @@ internal bool HasEnumeratorCancellationAttribute

#nullable enable

internal override DeclarationScope EffectiveScope
internal sealed override DeclarationScope EffectiveScope
{
get
{
Expand Down Expand Up @@ -1503,7 +1503,7 @@ internal SourceComplexParameterSymbol(
SyntaxReference syntaxRef,
bool isParams,
bool isExtensionMethodThis,
DeclarationScope? scope)
DeclarationScope scope)
: base(owner, ordinal, parameterType, refKind, name, locations, syntaxRef, isParams, isExtensionMethodThis, scope)
{
}
Expand All @@ -1526,7 +1526,7 @@ internal SourceComplexParameterSymbolWithCustomModifiersPrecedingRef(
SyntaxReference syntaxRef,
bool isParams,
bool isExtensionMethodThis,
DeclarationScope? scope)
DeclarationScope scope)
: base(owner, ordinal, parameterType, refKind, name, locations, syntaxRef, isParams, isExtensionMethodThis, scope)
{
Debug.Assert(!refCustomModifiers.IsEmpty);
Expand Down
11 changes: 4 additions & 7 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceParameterSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal abstract class SourceParameterSymbol : SourceParameterSymbolBase
private readonly string _name;
private readonly ImmutableArray<Location> _locations;
private readonly RefKind _refKind;
private readonly DeclarationScope? _scope;
private readonly DeclarationScope _scope;

public static SourceParameterSymbol Create(
Binder context,
Expand Down Expand Up @@ -101,7 +101,7 @@ protected SourceParameterSymbol(
TypeWithAnnotations parameterType,
int ordinal,
RefKind refKind,
DeclarationScope? scope,
DeclarationScope scope,
string name,
ImmutableArray<Location> locations)
: base(owner, ordinal)
Expand Down Expand Up @@ -225,16 +225,13 @@ public sealed override RefKind RefKind
/// <summary>
/// The declared scope. From source, this is from the <c>scope</c> keyword only.
/// </summary>
internal DeclarationScope? DeclaredScope => _scope;
internal DeclarationScope DeclaredScope => _scope;

internal abstract override DeclarationScope EffectiveScope { get; }

protected DeclarationScope CalculateEffectiveScopeIgnoringAttributes()
{
// DeclaredScope is only null in LambdaParameterSymbol where EffectiveScope can be provided rather than computed
Debug.Assert(this.DeclaredScope is not null);
var declaredScope = this.DeclaredScope.Value;

var declaredScope = this.DeclaredScope;
return declaredScope == DeclarationScope.Unscoped && ParameterHelpers.IsRefScopedByDefault(this) ?
DeclarationScope.RefScoped :
declaredScope;
Expand Down
49 changes: 49 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6950,5 +6950,54 @@ public DisplayAttribute() { }
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "Name").WithArguments("Name").WithLocation(5, 14)
);
}

[Fact]
public void DelegateConversions_ImplicitlyTypedParameter_RefParameter()
{
var source = """
struct R { }
delegate R D1(ref R r);
class Program
{
static void Main()
{
D1 d1 = r1 => r1; // 1
M(r2 => r2); // 2
}
static void M(D1 d1) { }
}
""";

var comp = CreateCompilation(source, targetFramework: TargetFramework.Net70);
comp.VerifyDiagnostics(
// (9,17): error CS1676: Parameter 1 must be declared with the 'ref' keyword
// D1 d1 = r1 => r1; // 1
Diagnostic(ErrorCode.ERR_BadParamRef, "r1").WithArguments("1", "ref").WithLocation(9, 17),
// (10,11): error CS1676: Parameter 1 must be declared with the 'ref' keyword
// M(r2 => r2); // 2
Diagnostic(ErrorCode.ERR_BadParamRef, "r2").WithArguments("1", "ref").WithLocation(10, 11)
);

var syntaxTree = comp.SyntaxTrees[0];
var root = syntaxTree.GetRoot();
var model = comp.GetSemanticModel(syntaxTree);

var lambdas = root.DescendantNodes().OfType<LambdaExpressionSyntax>().ToArray();

Assert.Equal("r1 => r1", lambdas[0].ToString());
var lambdaParameter1 = model.GetSymbolInfo(lambdas[0]).Symbol.GetParameters()[0];
Assert.Equal("? r1", lambdaParameter1.ToTestDisplayString());
Assert.Equal(RefKind.None, lambdaParameter1.RefKind);

// Implicitly-typed lambda parameters can get a type, but they cannot get a different ref-kind (or scoped-ness) during anonymous function conversion
// Tracked by https://github.com/dotnet/roslyn/issues/64985
Assert.Equal("r2 => r2", lambdas[1].ToString());
var lambdaParameter2 = model.GetSymbolInfo(lambdas[1]).Symbol.GetParameters()[0];
Assert.Equal("ref R r2", lambdaParameter2.ToTestDisplayString());
Assert.Equal(RefKind.Ref, lambdaParameter2.RefKind);
}
}
}
Loading

0 comments on commit c50d5e8

Please sign in to comment.