From 9fdc7d95292efd4a2fcf83dc61abb3c01ec57ede Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Tue, 12 Jan 2016 16:57:39 -0800 Subject: [PATCH] Remove support for inferring the return type of a local function. Fixes #7832 --- .../Portable/Binder/Binder_Statements.cs | 10 +- .../CSharp/Portable/Binder/InMethodBinder.cs | 2 +- .../Symbols/Source/LocalFunctionSymbol.cs | 144 +--------- .../Semantic/Semantics/LocalFunctionTests.cs | 272 ------------------ 4 files changed, 13 insertions(+), 415 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 183f3e252d090..0b3f1fdfd5a58 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -463,7 +463,7 @@ private BoundStatement BindLocalFunctionStatement(LocalFunctionStatementSyntax n if (block != null) { - localSymbol.ComputeReturnType(block, returnNullIfUnknown: false, isIterator: false); + localSymbol.ComputeReturnType(); // Have to do ControlFlowPass here because in MethodCompiler, we don't call this for synthed methods // rather we go directly to LowerBodyOrInitializer, which skips over flow analysis (which is in CompileMethod) @@ -2882,14 +2882,6 @@ protected bool IsGenericTaskReturningAsyncMethod() protected virtual TypeSymbol GetCurrentReturnType() { - // Local functions usually report an error when their return type is used recursively. (symbol.ReturnType creates an error type named "var") - // We actually want to return null here instead of an error type + diagnostic (for return type inference) - var containingLocalFunction = this.ContainingMemberOrLambda as LocalFunctionSymbol; - if (containingLocalFunction != null) - { - return containingLocalFunction.ReturnTypeNoForce; - } - return (this.ContainingMemberOrLambda as MethodSymbol)?.ReturnType; } diff --git a/src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs b/src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs index 17309ee2cffc7..de24df8f078fe 100644 --- a/src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs @@ -158,7 +158,7 @@ internal override GeneratedLabelSymbol ContinueLabel internal override TypeSymbol GetIteratorElementType(YieldStatementSyntax node, DiagnosticBag diagnostics) { - TypeSymbol returnType = (_methodSymbol as LocalFunctionSymbol)?.ReturnTypeIterator ?? _methodSymbol.ReturnType; + TypeSymbol returnType = _methodSymbol.ReturnType; if (!this.IsDirectlyInIterator) { diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs index 5684475bd5fca..d50977ad124d5 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs @@ -21,9 +21,9 @@ internal class LocalFunctionSymbol : MethodSymbol private ImmutableArray _parameters; private ImmutableArray _lazyTypeParameterConstraints; private TypeSymbol _returnType; - private bool _isVar; private bool _isVararg; private TypeSymbol _iteratorElementType; + // TODO: Find a better way to report diagnostics. // We can't put binding in the constructor, as it creates infinite recursion. // We can't report to Compilation.DeclarationDiagnostics as it's already too late and they will be dropped. @@ -61,8 +61,6 @@ public LocalFunctionSymbol( diagnostics.Add(ErrorCode.ERR_BadExtensionAgg, Locations[0]); } - _isVar = false; - _binder = binder; _diagnostics = diagnostics.ToReadOnlyAndFree(); } @@ -71,7 +69,7 @@ internal void GrabDiagnostics(DiagnosticBag addTo) { // force lazy init ComputeParameters(); - ComputeReturnType(body: null, returnNullIfUnknown: false, isIterator: false); + ComputeReturnType(); var diags = ImmutableInterlocked.InterlockedExchange(ref _diagnostics, default(ImmutableArray)); if (!diags.IsDefault) @@ -135,80 +133,19 @@ public override TypeSymbol ReturnType { get { - return ComputeReturnType(body: null, returnNullIfUnknown: false, isIterator: false); - } - } - - // Reason for ReturnTypeNoForce and ReturnTypeIterator: - // When computing the return type, sometimes we want to return null (ReturnTypeNoForce) instead of reporting a diagnostic - // or sometimes we want to disallow var and report an error about iterators (ReturnTypeIterator) - public TypeSymbol ReturnTypeNoForce - { - get - { - return ComputeReturnType(body: null, returnNullIfUnknown: true, isIterator: false); + return ComputeReturnType(); } } - public TypeSymbol ReturnTypeIterator - { - get - { - return ComputeReturnType(body: null, returnNullIfUnknown: false, isIterator: true); - } - } - - /* - Note: `var` return types are currently very broken in subtle ways, in particular in the IDE scenario when random things are being bound. - The basic problem is that a LocalFunctionSymbol needs to compute its return type, and to do that it needs access to its BoundBlock. - However, the BoundBlock needs access to the local function's return type. Recursion detection is tricky, because this property (.ReturnType) - doesn't have access to the binder where it is being accessed from (i.e. either from inside the local function, where it should report an error, - or from outside, where it should attempt to infer the return type from the block). - - The current (broken) system assumes that Binder_Statements.cs BindLocalFunctionStatement will always be called (and so a block will be provided) - before any (valid) uses of the local function are bound that require knowing the return type. This assumption breaks in the IDE, where - a use of a local function may be bound before BindLocalFunctionStatement is called on the corresponding local function. - */ - internal TypeSymbol ComputeReturnType(BoundBlock body, bool returnNullIfUnknown, bool isIterator) + internal TypeSymbol ComputeReturnType() { if (_returnType != null) { return _returnType; } + var diagnostics = DiagnosticBag.GetInstance(); - // we might call this multiple times if it's var. Only bind the first time, and cache if it's var. - TypeSymbol returnType = null; // guaranteed to be assigned, but compiler doesn't know that. - if (!_isVar) - { - bool isVar; - returnType = _binder.BindType(_syntax.ReturnType, diagnostics, out isVar); - _isVar = isVar; - } - if (_isVar) - { - if (isIterator) // cannot use IsIterator (the property) because that gets computed after the body is bound, which hasn't happened yet. - { - // Completely disallow use of var inferred in an iterator context. - // This is because we may have IAsyncEnumerable and similar types, which determine the type of state machine to emit. - // If we infer the return type, we won't know which state machine to generate. - returnType = _binder.CreateErrorType("var"); - // InMethodBinder reports ERR_BadIteratorReturn, so no need to report a diagnostic here. - } - else if (body == null) - { - if (returnNullIfUnknown) - { - diagnostics.Free(); - return null; - } - returnType = _binder.CreateErrorType("var"); - diagnostics.Add(ErrorCode.ERR_RecursivelyTypedVariable, _syntax.ReturnType.Location, this); - } - else - { - returnType = InferReturnType(body, diagnostics); - } - } + TypeSymbol returnType = _binder.BindType(_syntax.ReturnType, diagnostics); var raceReturnType = Interlocked.CompareExchange(ref _returnType, returnType, null); if (raceReturnType != null) { @@ -220,74 +157,15 @@ internal TypeSymbol ComputeReturnType(BoundBlock body, bool returnNullIfUnknown, // The return type of an async method must be void, Task or Task diagnostics.Add(ErrorCode.ERR_BadAsyncReturn, this.Locations[0]); } + // TODO: note there is a race condition here that will ultimately need to be fixed. + // Specifically, the Interlocked.CompareExchange above succeeds, and will be seen by + // other threads, before the diagnostics have been recorded in this symbol, below. + // We can resolve this with a lock around this method and any other methods that + // manipulate _diagnostics, directly or indirectly. AddDiagnostics(diagnostics.ToReadOnlyAndFree()); return returnType; } - private TypeSymbol InferReturnType(BoundBlock body, DiagnosticBag diagnostics) - { - int numberOfDistinctReturns; - var resultTypes = BoundLambda.BlockReturns.GetReturnTypes(body, out numberOfDistinctReturns); - if (numberOfDistinctReturns != resultTypes.Length) // included a "return;", no expression - { - resultTypes = resultTypes.Concat(ImmutableArray.Create((TypeSymbol)_binder.Compilation.GetSpecialType(SpecialType.System_Void))); - } - - TypeSymbol returnType; - if (resultTypes.IsDefaultOrEmpty) - { - returnType = _binder.Compilation.GetSpecialType(SpecialType.System_Void); - } - else if (resultTypes.Length == 1) - { - returnType = resultTypes[0]; - } - else - { - // Make sure every return type is exactly the same (not even a subclass of each other) - returnType = null; - foreach (var resultType in resultTypes) - { - if ((object)returnType == null) - { - returnType = resultType; - } - else if ((object)returnType != (object)resultType) - { - returnType = null; - break; - } - } - if (returnType == null) - { - returnType = _binder.CreateErrorType("var"); - diagnostics.Add(ErrorCode.ERR_ReturnTypesDontMatch, Locations[0], this); - return returnType; - } - } - - // do this before async lifting, as inferring Task is also not allowed. - if (returnType.SpecialType == SpecialType.System_Void) - { - diagnostics.Add(ErrorCode.ERR_CantInferVoid, Locations[0], this); - } - - if (IsAsync) - { - if (returnType.SpecialType == SpecialType.System_Void) - { - returnType = _binder.Compilation.GetWellKnownType(WellKnownType.System_Threading_Tasks_Task); - } - else - { - returnType = _binder.Compilation.GetWellKnownType(WellKnownType.System_Threading_Tasks_Task_T).Construct(returnType); - } - } - // cannot be iterator - - return returnType; - } - public override bool ReturnsVoid => ReturnType?.SpecialType == SpecialType.System_Void; public override int Arity => TypeParameters.Length; diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs index 1f14e2bc07611..daa4ff4cb4bc6 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs @@ -2308,19 +2308,6 @@ dynamic RetDyn() VerifyOutputInMain(source, "2", "System"); } - [Fact] - public void DynamicInferredReturn() - { - var source = @" -var RetDyn() -{ - return (dynamic)2; -} -Console.Write(RetDyn()); -"; - VerifyOutputInMain(source, "2", "System"); - } - [Fact] public void DynamicDelegate() { @@ -3469,265 +3456,6 @@ volatile void LocalVolatile() ); } - [Fact] - public void InferredReturnBasic() - { - var source = @" -var Local() -{ - return 2; -} -Console.Write(Local()); -"; - VerifyOutputInMain(source, "2", "System"); - } - - [Fact] - public void InferredReturnMultiple() - { - var source = @" -var Local(bool cond) -{ - if (cond) - { - return 2; - } - else - { - return 3; - } -} -Console.Write(Local(true)); -"; - VerifyOutputInMain(source, "2", "System"); - } - - [Fact] - public void InferredReturnNest() - { - var source = @" -var Local() -{ - var Inner() - { - return 2; - } - return Inner(); -} -Console.Write(Local()); -"; - VerifyOutputInMain(source, "2", "System"); - } - - [Fact] - public void InferredReturnVoid() - { - var source = @" -using System; - -class Program -{ - static void Main() - { - var Local() - { - Console.Write(2); - } - Local(); - } -} -"; - VerifyDiagnostics(source, - // (8,13): error CS8099: Cannot infer the type of 'Local()' as it does not return a value. - // var Local() - Diagnostic(ErrorCode.ERR_CantInferVoid, "Local").WithArguments("Local()").WithLocation(8, 13) - ); - } - - [Fact] - public void InferredReturnAsync() - { - var source = @" -async var Local() -{ - return await Task.FromResult(2); -} -Console.Write(Local().Result); -"; - VerifyOutputInMain(source, "2", "System", "System.Threading.Tasks"); - } - - [Fact] - public void InferredReturnTask() - { - var source = @" -using System; -using System.Threading.Tasks; - -class Program -{ - static void Main() - { - async var Local() - { - await Task.Yield(); - Console.WriteLine(2); - } - Local().Wait(); - } -} -"; - VerifyDiagnostics(source, - // (9,19): error CS8099: Cannot infer the type of 'Local()' as it does not return a value. - // async var Local() - Diagnostic(ErrorCode.ERR_CantInferVoid, "Local").WithArguments("Local()").WithLocation(9, 19) - ); - } - - [Fact] - public void BadInferredReturnRecursive() - { - var source = @" -class Program -{ - static void Main(string[] args) - { - var Local() - { - return Local() + 1; - } - } -} -"; - VerifyDiagnostics(source, - // (6,9): error CS7019: Type of 'Local()' cannot be inferred since its initializer directly or indirectly refers to the definition. - // var Local() - Diagnostic(ErrorCode.ERR_RecursivelyTypedVariable, "var").WithArguments("Local()").WithLocation(6, 9) - ); - } - - [Fact] - public void BadInferredReturnMutuallyRecursive() - { - var source = @" -class Program -{ - static void Main(string[] args) - { - var Local() - { - var Inner() - { - return Local(); - } - return Inner(); - } - } -} -"; - VerifyDiagnostics(source, - // (6,9): error CS7019: Type of 'Local()' cannot be inferred since its initializer directly or indirectly refers to the definition. - // var Local() - Diagnostic(ErrorCode.ERR_RecursivelyTypedVariable, "var").WithArguments("Local()").WithLocation(6, 9) - ); - } - - [Fact] - public void BadInferredReturnIteratorYield() - { - var source = @" -class Program -{ - static void Main(string[] args) - { - var Local() - { - yield return 2; - } - Local(); - } -} -"; - VerifyDiagnostics(source, - // (6,13): error CS1624: The body of 'Local()' cannot be an iterator block because 'var' is not an iterator interface type - // var Local() - Diagnostic(ErrorCode.ERR_BadIteratorReturn, "Local").WithArguments("Local()", "var").WithLocation(6, 13) - ); - } - - [Fact] - public void BadInferredReturnIteratorBreak() - { - var source = @" -class Program -{ - static void Main(string[] args) - { - var Local() - { - yield break; - } - Local(); - } -} -"; - VerifyDiagnostics(source, - // (6,13): error CS1624: The body of 'Local()' cannot be an iterator block because 'var' is not an iterator interface type - // var Local() - Diagnostic(ErrorCode.ERR_BadIteratorReturn, "Local").WithArguments("Local()", "var").WithLocation(6, 13) - ); - } - - [Fact] - public void InferredReturnMultipleReturn() - { - var source = @" -var Local(bool x) -{ - if (x) - { - return 2; - } - else - { - return 3; - } -} -Console.Write(Local(true)); -"; - VerifyOutputInMain(source, "2", "System"); - } - - [Fact] - public void BadInferredReturnDifferentTypes() - { - var source = @" -class Program -{ - static void Main(string[] args) - { - var Local(bool x) - { - if (x) - { - return 2; - } - else - { - return new object(); - } - } - Local(true); - } -} -"; - VerifyDiagnostics(source, - // (6,13): error CS8097: Cannot infer the return type of Local(bool) due to differing return types. - // var Local(bool x) - Diagnostic(ErrorCode.ERR_ReturnTypesDontMatch, "Local").WithArguments("Local(bool)").WithLocation(6, 13) - ); - } - [Fact] public void ArglistIterator() {