Skip to content

Commit

Permalink
Consistently return the same value from MethodSymbol.IteratorElementT…
Browse files Browse the repository at this point in the history
…ypeWithAnnotations API (#65291)

Fixes #40014.
  • Loading branch information
AlekseyTs authored Nov 9, 2022
1 parent 63b6b69 commit 072e9b4
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 113 deletions.
13 changes: 0 additions & 13 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1898,19 +1898,6 @@ private BoundBlock FinishBindBlockParts(CSharpSyntaxNode node, ImmutableArray<Bo
{
ImmutableArray<LocalSymbol> locals = GetDeclaredLocalsForScope(node);

if (IsDirectlyInIterator)
{
var method = ContainingMemberOrLambda as MethodSymbol;
if ((object)method != null)
{
method.IteratorElementTypeWithAnnotations = GetIteratorElementType();
}
else
{
Debug.Assert(diagnostics.DiagnosticBag is null || !diagnostics.DiagnosticBag.IsEmptyWithoutResolution);
}
}

return new BoundBlock(
node,
locals,
Expand Down
14 changes: 1 addition & 13 deletions src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ internal sealed class InMethodBinder : LocalScopeBinder
private MultiDictionary<string, ParameterSymbol> _lazyParameterMap;
private readonly MethodSymbol _methodSymbol;
private SmallDictionary<string, Symbol> _lazyDefinitionMap;
private TypeWithAnnotations.Boxed _iteratorElementType;

public InMethodBinder(MethodSymbol owner, Binder enclosing)
: base(enclosing, enclosing.Flags & ~BinderFlags.AllClearedAtExecutableCodeBoundary)
Expand Down Expand Up @@ -130,18 +129,7 @@ internal override TypeWithAnnotations GetIteratorElementType()
return !elementType.IsDefault ? elementType : TypeWithAnnotations.Create(CreateErrorType());
}

if (_iteratorElementType is null)
{
TypeWithAnnotations elementType = GetIteratorElementTypeFromReturnType(Compilation, refKind, returnType, errorLocation: null, diagnostics: null);
if (elementType.IsDefault)
{
elementType = TypeWithAnnotations.Create(CreateErrorType());
}

Interlocked.CompareExchange(ref _iteratorElementType, new TypeWithAnnotations.Boxed(elementType), null);
}

return _iteratorElementType.Value;
return _methodSymbol.IteratorElementTypeWithAnnotations;
}

internal static TypeWithAnnotations GetIteratorElementTypeFromReturnType(CSharpCompilation compilation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ internal SynthesizedClosureMethod(
originalMethod is LocalFunctionSymbol
? MakeName(topLevelMethod.Name, originalMethod.Name, topLevelMethodId, closureKind, lambdaId)
: MakeName(topLevelMethod.Name, topLevelMethodId, closureKind, lambdaId),
MakeDeclarationModifiers(closureKind, originalMethod))
MakeDeclarationModifiers(closureKind, originalMethod),
isIterator: originalMethod.IsIterator)
{
Debug.Assert(containingType.DeclaringCompilation is not null);

Expand Down
14 changes: 2 additions & 12 deletions src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,8 @@ private static bool BaseReferenceInReceiverWasRewritten([NotNullWhen(true)] Boun
private sealed partial class BaseMethodWrapperSymbol : SynthesizedMethodBaseSymbol
{
internal BaseMethodWrapperSymbol(NamedTypeSymbol containingType, MethodSymbol methodBeingWrapped, SyntaxNode syntax, string name)
: base(containingType, methodBeingWrapped, syntax.SyntaxTree.GetReference(syntax), syntax.GetLocation(), name, DeclarationModifiers.Private)
: base(containingType, methodBeingWrapped, syntax.SyntaxTree.GetReference(syntax), syntax.GetLocation(), name, DeclarationModifiers.Private,
isIterator: false)
{
Debug.Assert(containingType.ContainingModule is SourceModuleSymbol);
Debug.Assert(ReferenceEquals(methodBeingWrapped, methodBeingWrapped.ConstructedFrom));
Expand All @@ -724,17 +725,6 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r

AddSynthesizedAttribute(ref attributes, this.DeclaringCompilation.TrySynthesizeAttribute(WellKnownMember.System_Diagnostics_DebuggerHiddenAttribute__ctor));
}

internal override TypeWithAnnotations IteratorElementTypeWithAnnotations
{
get
{
// BaseMethodWrapperSymbol should not be rewritten by the IteratorRewriter
return default;
}
}

internal override bool IsIterator => false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ internal abstract class SynthesizedMethodBaseSymbol : SourceMemberMethodSymbol
private readonly string _name;
private ImmutableArray<TypeParameterSymbol> _typeParameters;
private ImmutableArray<ParameterSymbol> _parameters;
private TypeWithAnnotations.Boxed _iteratorElementType;

protected SynthesizedMethodBaseSymbol(NamedTypeSymbol containingType,
MethodSymbol baseMethod,
SyntaxReference syntaxReference,
Location location,
string name,
DeclarationModifiers declarationModifiers)
: base(containingType, syntaxReference, location, isIterator: false)
DeclarationModifiers declarationModifiers,
bool isIterator)
: base(containingType, syntaxReference, location, isIterator)
{
Debug.Assert((object)containingType != null);
Debug.Assert((object)baseMethod != null);
Expand Down Expand Up @@ -230,27 +230,5 @@ internal override bool IsExpressionBodied
{
get { return false; }
}

internal override TypeWithAnnotations IteratorElementTypeWithAnnotations
{
get
{
if (_iteratorElementType is null)
{
Interlocked.CompareExchange(ref _iteratorElementType,
new TypeWithAnnotations.Boxed(TypeMap.SubstituteType(BaseMethod.IteratorElementTypeWithAnnotations.Type)),
null);
}

return _iteratorElementType.Value;
}
set
{
Debug.Assert(!value.IsDefault);
Interlocked.Exchange(ref _iteratorElementType, new TypeWithAnnotations.Boxed(value));
}
}

internal override bool IsIterator => BaseMethod.IsIterator;
}
}
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1090,7 +1090,6 @@ internal virtual bool IsIterator
internal virtual TypeWithAnnotations IteratorElementTypeWithAnnotations
{
get { return default; }
set { throw ExceptionUtilities.Unreachable(); }
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;
using System.Threading;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal abstract class LocalFunctionOrSourceMemberMethodSymbol : SourceMethodSymbolWithAttributes
{
private TypeWithAnnotations.Boxed? _lazyIteratorElementType;

protected LocalFunctionOrSourceMemberMethodSymbol(SyntaxReference? syntaxReferenceOpt, bool isIterator)
: base(syntaxReferenceOpt)
{
if (isIterator)
{
_lazyIteratorElementType = TypeWithAnnotations.Boxed.Sentinel;
}
}

internal sealed override TypeWithAnnotations IteratorElementTypeWithAnnotations
{
get
{
if (_lazyIteratorElementType == TypeWithAnnotations.Boxed.Sentinel)
{
TypeWithAnnotations elementType = InMethodBinder.GetIteratorElementTypeFromReturnType(DeclaringCompilation, RefKind, ReturnType, errorLocation: null, diagnostics: null);

if (elementType.IsDefault)
{
elementType = TypeWithAnnotations.Create(new ExtendedErrorTypeSymbol(DeclaringCompilation, name: "", arity: 0, errorInfo: null, unreported: false));
}

Interlocked.CompareExchange(ref _lazyIteratorElementType, new TypeWithAnnotations.Boxed(elementType), TypeWithAnnotations.Boxed.Sentinel);

Debug.Assert(TypeSymbol.Equals(_lazyIteratorElementType.Value.Type, elementType.Type, TypeCompareKind.ConsiderEverything));
}

return _lazyIteratorElementType?.Value ?? default;
}
}

internal sealed override bool IsIterator => _lazyIteratorElementType is object;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal sealed class LocalFunctionSymbol : SourceMethodSymbolWithAttributes
internal sealed class LocalFunctionSymbol : LocalFunctionOrSourceMemberMethodSymbol
{
private readonly Binder _binder;
private readonly Symbol _containingSymbol;
Expand All @@ -26,7 +26,6 @@ internal sealed class LocalFunctionSymbol : SourceMethodSymbolWithAttributes
private ImmutableArray<ImmutableArray<TypeWithAnnotations>> _lazyTypeParameterConstraintTypes;
private ImmutableArray<TypeParameterConstraintKind> _lazyTypeParameterConstraintKinds;
private TypeWithAnnotations.Boxed? _lazyReturnType;
private TypeWithAnnotations.Boxed? _lazyIteratorElementType;

// Lock for initializing lazy fields and registering their diagnostics
// Acquire this lock when initializing lazy objects to guarantee their declaration
Expand All @@ -37,7 +36,7 @@ public LocalFunctionSymbol(
Binder binder,
Symbol containingSymbol,
LocalFunctionStatementSyntax syntax)
: base(syntax.GetReference())
: base(syntax.GetReference(), isIterator: SyntaxFacts.HasYieldOperations(syntax.Body))
{
Debug.Assert(containingSymbol.DeclaringCompilation == binder.Compilation);
_containingSymbol = containingSymbol;
Expand All @@ -48,11 +47,6 @@ public LocalFunctionSymbol(
DeclarationModifiers.Private |
syntax.Modifiers.ToDeclarationModifiers(diagnostics: _declarationDiagnostics.DiagnosticBag);

if (SyntaxFacts.HasYieldOperations(syntax.Body))
{
_lazyIteratorElementType = TypeWithAnnotations.Boxed.Sentinel;
}

this.CheckUnsafeModifier(_declarationModifiers, _declarationDiagnostics);

ScopeBinder = binder;
Expand Down Expand Up @@ -313,21 +307,6 @@ public override bool IsExtensionMethod
}
}

internal override TypeWithAnnotations IteratorElementTypeWithAnnotations
{
get
{
return _lazyIteratorElementType?.Value ?? default;
}
set
{
Debug.Assert(_lazyIteratorElementType == TypeWithAnnotations.Boxed.Sentinel || (_lazyIteratorElementType is object && TypeSymbol.Equals(_lazyIteratorElementType.Value.Type, value.Type, TypeCompareKind.ConsiderEverything2)));
Interlocked.CompareExchange(ref _lazyIteratorElementType, new TypeWithAnnotations.Boxed(value), TypeWithAnnotations.Boxed.Sentinel);
}
}

internal override bool IsIterator => _lazyIteratorElementType is object;

public override MethodKind MethodKind => MethodKind.LocalFunction;

public sealed override Symbol ContainingSymbol => _containingSymbol;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal abstract class SourceMemberMethodSymbol : SourceMethodSymbolWithAttributes, IAttributeTargetSymbol
internal abstract class SourceMemberMethodSymbol : LocalFunctionOrSourceMemberMethodSymbol, IAttributeTargetSymbol
{
// The flags type is used to compact many different bits of information.
protected struct Flags
Expand Down Expand Up @@ -195,7 +195,6 @@ public bool SetNullableContext(byte? value)

private readonly NamedTypeSymbol _containingType;
private ParameterSymbol _lazyThisParameter;
private TypeWithAnnotations.Boxed _lazyIteratorElementType;

private OverriddenOrHiddenMembersResult _lazyOverriddenOrHiddenMembers;

Expand Down Expand Up @@ -229,19 +228,14 @@ protected SourceMemberMethodSymbol(
SyntaxReference syntaxReferenceOpt,
ImmutableArray<Location> locations,
bool isIterator)
: base(syntaxReferenceOpt)
: base(syntaxReferenceOpt, isIterator)
{
Debug.Assert((object)containingType != null);
Debug.Assert(!locations.IsEmpty);
Debug.Assert(containingType.DeclaringCompilation is not null);

_containingType = containingType;
this.locations = locations;

if (isIterator)
{
_lazyIteratorElementType = TypeWithAnnotations.Boxed.Sentinel;
}
}

protected void CheckEffectiveAccessibility(TypeWithAnnotations returnType, ImmutableArray<ParameterSymbol> parameters, BindingDiagnosticBag diagnostics)
Expand Down Expand Up @@ -723,21 +717,6 @@ internal sealed override bool TryGetThisParameter(out ParameterSymbol thisParame
return true;
}

internal override TypeWithAnnotations IteratorElementTypeWithAnnotations
{
get
{
return _lazyIteratorElementType?.Value ?? default;
}
set
{
Debug.Assert(_lazyIteratorElementType == TypeWithAnnotations.Boxed.Sentinel || TypeSymbol.Equals(_lazyIteratorElementType.Value.Type, value.Type, TypeCompareKind.ConsiderEverything2));
Interlocked.CompareExchange(ref _lazyIteratorElementType, new TypeWithAnnotations.Boxed(value), TypeWithAnnotations.Boxed.Sentinel);
}
}

internal override bool IsIterator => _lazyIteratorElementType is object;

//overridden appropriately in SourceMemberMethodSymbol
public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
{
Expand Down
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/IteratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,15 @@ IEnumerable<int> I()
}
}";
var comp = CreateCompilation(text);

var i = comp.GetMember<MethodSymbol>("Test.I");
Assert.True(i.IsIterator);
Assert.Equal("System.Int32", i.IteratorElementTypeWithAnnotations.ToTestDisplayString());

comp.VerifyDiagnostics();

Assert.True(i.IsIterator);
Assert.Equal("System.Int32", i.IteratorElementTypeWithAnnotations.ToTestDisplayString());
}

[Fact]
Expand Down
18 changes: 16 additions & 2 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2366,8 +2366,22 @@ public unsafe IEnumerable<int> M4(int* a)
})();
}
}";
CreateCompilation(src, options: TestOptions.UnsafeDebugDll)
.VerifyDiagnostics(
var comp = CreateCompilation(src, options: TestOptions.UnsafeDebugDll, parseOptions: TestOptions.RegularNext.WithFeature("run-nullable-analysis", "never"));

var tree = comp.SyntaxTrees.Single();
var model = comp.GetSemanticModel(tree);
LocalFunctionStatementSyntax declaration = tree.GetRoot().DescendantNodes().OfType<LocalFunctionStatementSyntax>().First();
var local = model.GetDeclaredSymbol(declaration).GetSymbol<MethodSymbol>();

Assert.True(local.IsIterator);
Assert.Equal("System.Int32", local.IteratorElementTypeWithAnnotations.ToTestDisplayString());

model.GetOperation(declaration.Body);

Assert.True(local.IsIterator);
Assert.Equal("System.Int32", local.IteratorElementTypeWithAnnotations.ToTestDisplayString());

comp.VerifyDiagnostics(
// (8,37): error CS1637: Iterators cannot have unsafe parameters or yield types
// IEnumerable<int> Local(int* a) { yield break; }
Diagnostic(ErrorCode.ERR_UnsafeIteratorArgType, "a").WithLocation(8, 37),
Expand Down

0 comments on commit 072e9b4

Please sign in to comment.