diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.cs b/src/Compilers/CSharp/Portable/Binder/Binder.cs index 970011bce940a..38be46f53f785 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.cs @@ -317,13 +317,10 @@ internal virtual GeneratedLabelSymbol ContinueLabel /// /// Get the element type of this iterator. /// - /// Node to report diagnostics, if any, such as "yield statement cannot be used - /// inside a lambda expression" - /// Where to place any diagnostics /// Element type of the current iterator, or an error type. - internal virtual TypeWithAnnotations GetIteratorElementType(YieldStatementSyntax node, DiagnosticBag diagnostics) + internal virtual TypeWithAnnotations GetIteratorElementType() { - return Next.GetIteratorElementType(node, diagnostics); + return Next.GetIteratorElementType(); } /// diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index df7477a72051e..6cdb47bca4e90 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -207,19 +207,23 @@ private void CheckRequiredLangVersionForAsyncIteratorMethods(DiagnosticBag diagn } } - private BoundStatement BindYieldReturnStatement(YieldStatementSyntax node, DiagnosticBag diagnostics) + protected virtual void ValidateYield(YieldStatementSyntax node, DiagnosticBag diagnostics) { - var binder = this; + Next?.ValidateYield(node, diagnostics); + } - TypeSymbol elementType = binder.GetIteratorElementType(node, diagnostics).Type; + private BoundStatement BindYieldReturnStatement(YieldStatementSyntax node, DiagnosticBag diagnostics) + { + ValidateYield(node, diagnostics); + TypeSymbol elementType = GetIteratorElementType().Type; BoundExpression argument = (node.Expression == null) ? BadExpression(node).MakeCompilerGenerated() - : binder.BindValue(node.Expression, diagnostics, BindValueKind.RValue); + : BindValue(node.Expression, diagnostics, BindValueKind.RValue); argument = ValidateEscape(argument, ExternalScope, isByRef: false, diagnostics: diagnostics); if (!argument.HasAnyErrors) { - argument = binder.GenerateConversionForAssignment(elementType, argument, diagnostics); + argument = GenerateConversionForAssignment(elementType, argument, diagnostics); } else { @@ -261,7 +265,7 @@ private BoundStatement BindYieldBreakStatement(YieldStatementSyntax node, Diagno Error(diagnostics, ErrorCode.ERR_YieldNotAllowedInScript, node.YieldKeyword); } - GetIteratorElementType(node, diagnostics); + ValidateYield(node, diagnostics); CheckRequiredLangVersionForAsyncIteratorMethods(diagnostics); return new BoundYieldBreakStatement(node); } @@ -1706,7 +1710,7 @@ private BoundBlock BindBlockParts(BlockSyntax node, DiagnosticBag diagnostics) var method = ContainingMemberOrLambda as MethodSymbol; if ((object)method != null) { - method.IteratorElementTypeWithAnnotations = GetIteratorElementType(null, diagnostics); + method.IteratorElementTypeWithAnnotations = GetIteratorElementType(); } else { diff --git a/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs b/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs index 9ab0f461fb81e..7ed830e5d33ca 100644 --- a/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs @@ -143,7 +143,7 @@ internal override BoundExpression ConditionalReceiverExpression // This should only be called in the context of syntactically incorrect programs. In other // contexts statements are surrounded by some enclosing method or lambda. - internal override TypeWithAnnotations GetIteratorElementType(YieldStatementSyntax node, DiagnosticBag diagnostics) + internal override TypeWithAnnotations GetIteratorElementType() { // There's supposed to be an enclosing method or lambda. throw ExceptionUtilities.Unreachable; diff --git a/src/Compilers/CSharp/Portable/Binder/ExecutableCodeBinder.cs b/src/Compilers/CSharp/Portable/Binder/ExecutableCodeBinder.cs index f7cd439b1b321..25e38a3790ea1 100644 --- a/src/Compilers/CSharp/Portable/Binder/ExecutableCodeBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/ExecutableCodeBinder.cs @@ -155,16 +155,39 @@ public void ValidateIteratorMethods(DiagnosticBag diagnostics) } } + Location errorLocation = iterator.Locations[0]; if (iterator.IsVararg) { // error CS1636: __arglist is not allowed in the parameter list of iterators - diagnostics.Add(ErrorCode.ERR_VarargsIterator, iterator.Locations[0]); + diagnostics.Add(ErrorCode.ERR_VarargsIterator, errorLocation); } if (((iterator as SourceMemberMethodSymbol)?.IsUnsafe == true || (iterator as LocalFunctionSymbol)?.IsUnsafe == true) && Compilation.Options.AllowUnsafe) // Don't cascade { - diagnostics.Add(ErrorCode.ERR_IllegalInnerUnsafe, iterator.Locations[0]); + diagnostics.Add(ErrorCode.ERR_IllegalInnerUnsafe, errorLocation); + } + + var returnType = iterator.ReturnType; + RefKind refKind = iterator.RefKind; + TypeWithAnnotations elementType = InMethodBinder.GetIteratorElementTypeFromReturnType(Compilation, refKind, returnType, errorLocation, diagnostics); + + if (elementType.IsDefault) + { + if (refKind != RefKind.None) + { + Error(diagnostics, ErrorCode.ERR_BadIteratorReturnRef, errorLocation, iterator); + } + else if (!returnType.IsErrorType()) + { + Error(diagnostics, ErrorCode.ERR_BadIteratorReturn, errorLocation, iterator, returnType); + } + } + + bool asyncInterface = InMethodBinder.IsAsyncStreamInterface(Compilation, refKind, returnType); + if (asyncInterface && !iterator.IsAsync) + { + diagnostics.Add(ErrorCode.ERR_IteratorMustBeAsync, errorLocation, iterator, returnType); } } } diff --git a/src/Compilers/CSharp/Portable/Binder/InContainerBinder.cs b/src/Compilers/CSharp/Portable/Binder/InContainerBinder.cs index a303003736b93..50057c090fea9 100644 --- a/src/Compilers/CSharp/Portable/Binder/InContainerBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/InContainerBinder.cs @@ -234,7 +234,7 @@ internal override void GetCandidateExtensionMethods( } } - internal override TypeWithAnnotations GetIteratorElementType(YieldStatementSyntax node, DiagnosticBag diagnostics) + internal override TypeWithAnnotations GetIteratorElementType() { if (IsScriptClass) { @@ -245,7 +245,7 @@ internal override TypeWithAnnotations GetIteratorElementType(YieldStatementSynta else { // This path would eventually throw, if we didn't have the case above. - return Next.GetIteratorElementType(node, diagnostics); + return Next.GetIteratorElementType(); } } diff --git a/src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs b/src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs index 4ca2ddcddeece..8fe6ee7478a86 100644 --- a/src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs @@ -13,27 +13,15 @@ namespace Microsoft.CodeAnalysis.CSharp /// /// A binder for a method body, which places the method's parameters in scope /// and notes if the method is an iterator method. + /// Note: instances of this type can be re-used across different attempts at compiling the same method (caching by binder factory). /// internal sealed class InMethodBinder : LocalScopeBinder { private MultiDictionary _lazyParameterMap; private readonly MethodSymbol _methodSymbol; private SmallDictionary _lazyDefinitionMap; - private IteratorInfo _iteratorInfo; - - private class IteratorInfo - { - public static readonly IteratorInfo Empty = new IteratorInfo(default, default(ImmutableArray)); - - public readonly TypeWithAnnotations ElementType; - public readonly ImmutableArray ElementTypeDiagnostics; - - public IteratorInfo(TypeWithAnnotations elementType, ImmutableArray elementTypeDiagnostics) - { - this.ElementType = elementType; - this.ElementTypeDiagnostics = elementTypeDiagnostics; - } - } + private TypeWithAnnotations.Boxed _iteratorElementType; + private readonly static TypeWithAnnotations.Boxed SentinelElementType = new TypeWithAnnotations.Boxed(default); public InMethodBinder(MethodSymbol owner, Binder enclosing) : base(enclosing, enclosing.Flags & ~BinderFlags.AllClearedAtExecutableCodeBoundary) @@ -88,9 +76,9 @@ internal override bool IsInMethodBody internal void MakeIterator() { - if (_iteratorInfo == null) + if (_iteratorElementType == null) { - _iteratorInfo = IteratorInfo.Empty; + _iteratorElementType = SentinelElementType; } } @@ -98,7 +86,7 @@ internal override bool IsDirectlyInIterator { get { - return _iteratorInfo != null; + return _iteratorElementType != null; } } @@ -126,7 +114,11 @@ internal override GeneratedLabelSymbol ContinueLabel } } - internal override TypeWithAnnotations GetIteratorElementType(YieldStatementSyntax node, DiagnosticBag diagnostics) + protected override void ValidateYield(YieldStatementSyntax node, DiagnosticBag diagnostics) + { + } + + internal override TypeWithAnnotations GetIteratorElementType() { RefKind refKind = _methodSymbol.RefKind; TypeSymbol returnType = _methodSymbol.ReturnType; @@ -139,49 +131,26 @@ internal override TypeWithAnnotations GetIteratorElementType(YieldStatementSynta // and deduce an iterator element type from the return type. If we didn't do this, the // TypeInfo.ConvertedType of the yield statement would always be an error type. However, we will // not mutate any state (i.e. we won't store the result). - var elementType = GetIteratorElementTypeFromReturnType(Compilation, refKind, returnType, node, diagnostics).elementType; + var elementType = GetIteratorElementTypeFromReturnType(Compilation, refKind, returnType, errorLocation: null, diagnostics: null); return !elementType.IsDefault ? elementType : TypeWithAnnotations.Create(CreateErrorType()); } - if (_iteratorInfo == IteratorInfo.Empty) + if (_iteratorElementType == SentinelElementType) { - DiagnosticBag elementTypeDiagnostics = DiagnosticBag.GetInstance(); - - (TypeWithAnnotations elementType, bool asyncInterface) = GetIteratorElementTypeFromReturnType(Compilation, refKind, returnType, node, elementTypeDiagnostics); - - Location errorLocation = _methodSymbol.Locations[0]; + TypeWithAnnotations elementType = GetIteratorElementTypeFromReturnType(Compilation, refKind, returnType, errorLocation: null, diagnostics: null); if (elementType.IsDefault) { - if (refKind != RefKind.None) - { - Error(elementTypeDiagnostics, ErrorCode.ERR_BadIteratorReturnRef, errorLocation, _methodSymbol); - } - else if (!returnType.IsErrorType()) - { - Error(elementTypeDiagnostics, ErrorCode.ERR_BadIteratorReturn, errorLocation, _methodSymbol, returnType); - } elementType = TypeWithAnnotations.Create(CreateErrorType()); } - else if (asyncInterface && !_methodSymbol.IsAsync) - { - Error(elementTypeDiagnostics, ErrorCode.ERR_IteratorMustBeAsync, errorLocation, _methodSymbol, returnType); - } - - var info = new IteratorInfo(elementType, elementTypeDiagnostics.ToReadOnlyAndFree()); - var oldInfo = Interlocked.CompareExchange(ref _iteratorInfo, info, IteratorInfo.Empty); - if (oldInfo == IteratorInfo.Empty) - { - diagnostics.AddRange(_iteratorInfo.ElementTypeDiagnostics); - } + Interlocked.CompareExchange(ref _iteratorElementType, new TypeWithAnnotations.Boxed(elementType), SentinelElementType); } - return _iteratorInfo.ElementType; + return _iteratorElementType.Value; } - // If an element type is found, we also return whether the interface is meant to be used with async. - internal static (TypeWithAnnotations elementType, bool asyncInterface) GetIteratorElementTypeFromReturnType(CSharpCompilation compilation, - RefKind refKind, TypeSymbol returnType, CSharpSyntaxNode errorLocationNode, DiagnosticBag diagnostics) + internal static TypeWithAnnotations GetIteratorElementTypeFromReturnType(CSharpCompilation compilation, + RefKind refKind, TypeSymbol returnType, Location errorLocation, DiagnosticBag diagnostics) { if (refKind == RefKind.None && returnType.Kind == SymbolKind.NamedType) { @@ -193,25 +162,41 @@ internal static (TypeWithAnnotations elementType, bool asyncInterface) GetIterat var objectType = compilation.GetSpecialType(SpecialType.System_Object); if (diagnostics != null) { - ReportUseSiteDiagnostics(objectType, diagnostics, errorLocationNode); + ReportUseSiteDiagnostics(objectType, diagnostics, errorLocation); } - return (TypeWithAnnotations.Create(objectType), false); + return TypeWithAnnotations.Create(objectType); case SpecialType.System_Collections_Generic_IEnumerable_T: case SpecialType.System_Collections_Generic_IEnumerator_T: - return (((NamedTypeSymbol)returnType).TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0], false); + return ((NamedTypeSymbol)returnType).TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0]; } - if (TypeSymbol.Equals(originalDefinition, compilation.GetWellKnownType(WellKnownType.System_Collections_Generic_IAsyncEnumerable_T), TypeCompareKind.ConsiderEverything2) || - TypeSymbol.Equals(originalDefinition, compilation.GetWellKnownType(WellKnownType.System_Collections_Generic_IAsyncEnumerator_T), TypeCompareKind.ConsiderEverything2)) + if (TypeSymbol.Equals(originalDefinition, compilation.GetWellKnownType(WellKnownType.System_Collections_Generic_IAsyncEnumerable_T), TypeCompareKind.ConsiderEverything) || + TypeSymbol.Equals(originalDefinition, compilation.GetWellKnownType(WellKnownType.System_Collections_Generic_IAsyncEnumerator_T), TypeCompareKind.ConsiderEverything)) { - return (((NamedTypeSymbol)returnType).TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0], true); + return ((NamedTypeSymbol)returnType).TypeArgumentsWithAnnotationsNoUseSiteDiagnostics[0]; } } return default; } + internal static bool IsAsyncStreamInterface(CSharpCompilation compilation, RefKind refKind, TypeSymbol returnType) + { + if (refKind == RefKind.None && returnType.Kind == SymbolKind.NamedType) + { + TypeSymbol originalDefinition = returnType.OriginalDefinition; + + if (TypeSymbol.Equals(originalDefinition, compilation.GetWellKnownType(WellKnownType.System_Collections_Generic_IAsyncEnumerable_T), TypeCompareKind.ConsiderEverything) || + TypeSymbol.Equals(originalDefinition, compilation.GetWellKnownType(WellKnownType.System_Collections_Generic_IAsyncEnumerator_T), TypeCompareKind.ConsiderEverything)) + { + return true; + } + } + + return false; + } + internal override void LookupSymbolsInSingleBinder( LookupResult result, string name, int arity, ConsList basesBeingResolved, LookupOptions options, Binder originalBinder, bool diagnose, ref HashSet useSiteDiagnostics) { diff --git a/src/Compilers/CSharp/Portable/Binder/WithLambdaParametersBinder.cs b/src/Compilers/CSharp/Portable/Binder/WithLambdaParametersBinder.cs index 4422e9b0a9142..356b5208d90a5 100644 --- a/src/Compilers/CSharp/Portable/Binder/WithLambdaParametersBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/WithLambdaParametersBinder.cs @@ -71,13 +71,17 @@ internal override bool IsDirectlyInIterator // NOTE: Specifically not overriding IsIndirectlyInIterator. - internal override TypeWithAnnotations GetIteratorElementType(YieldStatementSyntax node, DiagnosticBag diagnostics) + internal override TypeWithAnnotations GetIteratorElementType() + { + return TypeWithAnnotations.Create(CreateErrorType()); + } + + protected override void ValidateYield(YieldStatementSyntax node, DiagnosticBag diagnostics) { if (node != null) { diagnostics.Add(ErrorCode.ERR_YieldInAnonMeth, node.YieldKeyword.GetLocation()); } - return TypeWithAnnotations.Create(CreateErrorType()); } internal override void LookupSymbolsInSingleBinder( diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index f2ad5444fe9a9..e850c00c915e6 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -7758,7 +7758,7 @@ public override BoundNode VisitYieldReturnStatement(BoundYieldReturnStatement no } var method = _delegateInvokeMethod ?? (MethodSymbol)_symbol; TypeWithAnnotations elementType = InMethodBinder.GetIteratorElementTypeFromReturnType(compilation, RefKind.None, - method.ReturnType, errorLocationNode: null, diagnostics: null).elementType; + method.ReturnType, errorLocation: null, diagnostics: null); _ = VisitOptionalImplicitConversion(expr, elementType, useLegacyWarnings: false, trackMembers: false, AssignmentKind.Return); return null; diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs index 7ae5dfe43188b..fb77bef571015 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs @@ -1881,6 +1881,7 @@ public static async System.Collections.Generic.IAsyncEnumerable M() [Fact] [WorkItem(31608, "https://github.com/dotnet/roslyn/issues/31608")] + [WorkItem(39970, "https://github.com/dotnet/roslyn/issues/39970")] public void AsyncIterator_WithoutAwait_WithoutAsync() { string source = @" @@ -1892,11 +1893,13 @@ static System.Collections.Generic.IAsyncEnumerable M() } }"; var comp = CreateCompilationWithAsyncIterator(source); - comp.VerifyDiagnostics( + var expected = new DiagnosticDescription[] { // (4,61): error CS8403: Method 'C.M()' with an iterator block must be 'async' to return 'IAsyncEnumerable' // static System.Collections.Generic.IAsyncEnumerable M() Diagnostic(ErrorCode.ERR_IteratorMustBeAsync, "M").WithArguments("C.M()", "System.Collections.Generic.IAsyncEnumerable").WithLocation(4, 61) - ); + }; + comp.VerifyDiagnostics(expected); + comp.VerifyEmitDiagnostics(expected); } [Fact] diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs index 569fd1d9354da..0f465eb4131a0 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs @@ -14586,14 +14586,16 @@ public static void Main() } "; var comp = CreateCompilation(text); - comp.VerifyDiagnostics( + var expected = new DiagnosticDescription[] { // (12,13): error CS1621: The yield statement cannot be used inside an anonymous method or lambda expression // yield return this; // CS1621 Diagnostic(ErrorCode.ERR_YieldInAnonMeth, "yield"), // (8,24): error CS0161: 'C.GetEnumerator()': not all code paths return a value // public IEnumerator GetEnumerator() Diagnostic(ErrorCode.ERR_ReturnExpected, "GetEnumerator").WithArguments("C.GetEnumerator()") - ); + }; + comp.VerifyDiagnostics(expected); + comp.VerifyEmitDiagnostics(expected); } [Fact] diff --git a/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs b/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs index 305a6bda5766e..ccc139fe48e2d 100644 --- a/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs +++ b/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs @@ -2249,6 +2249,35 @@ public static int Main() Assert.Equal(1, compilation.GetDiagnostics().Length); } + [WorkItem(39992, "https://github.com/dotnet/roslyn/issues/39992")] + [Fact] + public void GetDiagnosticsCalledTwice_GetEmitDiagnostics() + { + var text = @" +interface IMyEnumerator { } + +public class Test +{ + static IMyEnumerator Goo() + { + yield break; + } + + public static int Main() + { + return 1; + } +}"; + var compilation = CreateCompilation(text); + var expected = new DiagnosticDescription[] { + // (6,26): error CS1624: The body of 'Test.Goo()' cannot be an iterator block because 'IMyEnumerator' is not an iterator interface type + // static IMyEnumerator Goo() + Diagnostic(ErrorCode.ERR_BadIteratorReturn, "Goo").WithArguments("Test.Goo()", "IMyEnumerator").WithLocation(6, 26) + }; + compilation.VerifyDiagnostics(expected); + compilation.VerifyEmitDiagnostics(expected); + } + [Fact] public void TestArgumentEquality() {