From 12717c54c192b17e463a00dfcd5af0ba6b17bc0e Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Wed, 9 Nov 2016 12:11:23 -0800 Subject: [PATCH] Use the binder that owns local function scope for name conflict errors (#14837) The definite assignment errors were fixed by changing the design of local functions to allow forward calls to local functions in scope. Fixes #13029 --- .../CSharp/Portable/Binder/Binder.cs | 2 +- .../Portable/Binder/Binder_Statements.cs | 24 +-- .../Portable/Binder/LocalBinderFactory.cs | 17 +- .../Symbols/Source/LocalFunctionSymbol.cs | 19 +- .../Semantic/Semantics/LocalFunctionTests.cs | 162 ++++++++++++++++++ .../Semantics/PatternMatchingTests_Scope.cs | 55 ++++++ 6 files changed, 237 insertions(+), 42 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.cs b/src/Compilers/CSharp/Portable/Binder/Binder.cs index cf6d869686d..3e785edb8d4 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.cs @@ -145,7 +145,7 @@ internal bool CheckOverflowAtCompileTime } /// - /// Some nodes have special binder's for their contents (like Block's) + /// Some nodes have special binders for their contents (like Blocks) /// internal virtual Binder GetBinder(SyntaxNode node) { diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 316f534fc9c..5294354a071 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -443,30 +443,12 @@ private BoundStatement BindGoto(GotoStatementSyntax node, DiagnosticBag diagnost } private BoundStatement BindLocalFunctionStatement(LocalFunctionStatementSyntax node, DiagnosticBag diagnostics) - { - var binder = GetBinder(node); - Debug.Assert(binder != null); - - return binder.BindLocalFunctionStatementParts(node, diagnostics); - } - - private BoundStatement BindLocalFunctionStatementParts(LocalFunctionStatementSyntax node, DiagnosticBag diagnostics) { // already defined symbol in containing block var localSymbol = this.LookupLocalFunction(node.Identifier); - var hasErrors = false; - - // In error scenarios with misplaced code, it is possible we can't bind the local declaration. - // This occurs through the semantic model. In that case concoct a plausible result. - if (localSymbol == null) - { - localSymbol = new LocalFunctionSymbol(this, this.ContainingMemberOrLambda, node); - } - else - { - hasErrors |= this.ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics); - } + var hasErrors = localSymbol.ScopeBinder + .ValidateDeclarationNameConflictsInScope(localSymbol, diagnostics); BoundBlock block; if (node.Body != null) @@ -481,7 +463,7 @@ private BoundStatement BindLocalFunctionStatementParts(LocalFunctionStatementSyn { block = null; hasErrors = true; - // TODO: add a message for this? + // TODO(https://github.com/dotnet/roslyn/issues/14900): Better error message diagnostics.Add(ErrorCode.ERR_ConcreteMissingBody, localSymbol.Locations[0], localSymbol); } diff --git a/src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs b/src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs index 2d685410aef..ee34bab9697 100644 --- a/src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs +++ b/src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs @@ -223,21 +223,14 @@ public override void VisitLocalFunctionStatement(LocalFunctionStatementSyntax no { var oldMethod = _containingMemberOrLambda; _containingMemberOrLambda = match; - Binder addToMap; - if (match.IsGenericMethod) - { - addToMap = new WithMethodTypeParametersBinder(match, _enclosing); - } - else - { - addToMap = _enclosing; - } - - AddToMap(node, addToMap); if (body != null) { - Visit(body, new InMethodBinder(match, addToMap)); + Binder binder = match.IsGenericMethod + ? new WithMethodTypeParametersBinder(match, _enclosing) + : _enclosing; + + Visit(body, new InMethodBinder(match, binder)); } _containingMemberOrLambda = oldMethod; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs index 2f3cd6d74ae..08778746959 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs @@ -98,6 +98,15 @@ internal ReturnTypeAndDiagnostics(TypeSymbol returnType, ImmutableArray + /// Binder that owns the scope for the local function symbol, namely the scope where the + /// local function is declared. + /// + internal Binder ScopeBinder => _syntax.TypeParameterList == null + ? _binder + : _binder.Next; // If there are type parameters, this binder is wrapped + // in a WithMethodTypeParametersBinder + internal void GrabDiagnostics(DiagnosticBag addTo) { // force lazy init @@ -328,15 +337,9 @@ private ImmutableArray MakeTypeParameters(DiagnosticBag dia var location = identifier.GetLocation(); var name = identifier.ValueText; - // TODO: Add diagnostic checks for nested local functions (and containing method) - if (name == this.Name) - { - diagnostics.Add(ErrorCode.ERR_TypeVariableSameAsParent, location, name); - } - - for (int i = 0; i < result.Count; i++) + foreach (var @param in result) { - if (name == result[i].Name) + if (name == @param.Name) { diagnostics.Add(ErrorCode.ERR_DuplicateTypeParameter, location, name); break; diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs index d3d8f0fd0e5..3581895bdfa 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs @@ -29,6 +29,168 @@ internal void VerifyDiagnostics(string source, CSharpCompilationOptions options, [CompilerTrait(CompilerFeature.LocalFunctions)] public class LocalFunctionTests : LocalFunctionsTestBase { + [Fact] + public void TypeParameterBindingScope() + { + var src = @" +class C +{ + public void M() + { + { + int T = 0; // Should not have error + + int Local() => 0; // Should conflict with above + Local(); + T++; + } + { + int T() => 0; + T(); + } + { + int Local() => 0; + Local(); + } + } + public void M2() + { + { + int Local() => 0; + Local(); + } + { + int Local1() + { + int Local2() => 0; + return Local2(); + } + Local1(); + } + { + int T() => 0; + T(); + } + { + int Local1() + { + int V() => 0; + return V(); + } + Local1(); + } + { + int Local1() + { + int Local2() + { + // Conflicts with method type parameter + int T() => 0; + return T(); + } + return Local2(); + } + Local1(); + } + { + int Local1() + { + int Local2() + { + // Shadows M.2 + int Local3() => 0; + return Local3(); + } + return Local2(); + } + Local1(); + } + } + public void V() { } +} +"; + var comp = CreateCompilationWithMscorlib(src); + comp.VerifyDiagnostics( + // (9,23): error CS0136: A local or parameter named 'T' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // int Local() => 0; // Should conflict with above + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "T").WithArguments("T").WithLocation(9, 23), + // (14,19): error CS0136: A local or parameter named 'T' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // int T() => 0; + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "T").WithArguments("T").WithLocation(14, 19), + // (18,26): error CS0692: Duplicate type parameter 'T' + // int Local() => 0; + Diagnostic(ErrorCode.ERR_DuplicateTypeParameter, "T").WithArguments("T").WithLocation(18, 26), + // (25,23): warning CS0693: Type parameter 'T' has the same name as the type parameter from outer type 'C.M2()' + // int Local() => 0; + Diagnostic(ErrorCode.WRN_TypeParameterSameAsOuterTypeParameter, "T").WithArguments("T", "C.M2()").WithLocation(25, 23), + // (31,28): warning CS0693: Type parameter 'V' has the same name as the type parameter from outer type 'Local1()' + // int Local2() => 0; + Diagnostic(ErrorCode.WRN_TypeParameterSameAsOuterTypeParameter, "V").WithArguments("V", "Local1()").WithLocation(31, 28), + // (37,17): error CS0412: 'T': a parameter, local variable, or local function cannot have the same name as a method type parameter + // int T() => 0; + Diagnostic(ErrorCode.ERR_LocalSameNameAsTypeParam, "T").WithArguments("T").WithLocation(37, 17), + // (43,21): error CS0412: 'V': a parameter, local variable, or local function cannot have the same name as a method type parameter + // int V() => 0; + Diagnostic(ErrorCode.ERR_LocalSameNameAsTypeParam, "V").WithArguments("V").WithLocation(43, 21), + // (54,25): error CS0412: 'T': a parameter, local variable, or local function cannot have the same name as a method type parameter + // int T() => 0; + Diagnostic(ErrorCode.ERR_LocalSameNameAsTypeParam, "T").WithArguments("T").WithLocation(54, 25), + // (67,32): warning CS0693: Type parameter 'T' has the same name as the type parameter from outer type 'C.M2()' + // int Local3() => 0; + Diagnostic(ErrorCode.WRN_TypeParameterSameAsOuterTypeParameter, "T").WithArguments("T", "C.M2()").WithLocation(67, 32)); + } + + [Fact] + public void LocalFuncAndTypeParameterOnType() + { + var comp = CreateCompilationWithMscorlib(@" +class C2 +{ + public void M() + { + { + int Local1() + { + int Local2() => 0; + return Local2(); + } + Local1(); + } + { + int Local1() + { + int Local2() + { + // Shadows type parameter + int T() => 0; + + // Type parameter resolves in type only context + T t = default(T); + + // Ambiguous context chooses local + T.M(); + + // Call chooses local + return T(); + } + return Local2(); + } + Local1(); + } + } +}"); + comp.VerifyDiagnostics( + // (9,28): warning CS0693: Type parameter 'T' has the same name as the type parameter from outer type 'C2' + // int Local2() => 0; + Diagnostic(ErrorCode.WRN_TypeParameterSameAsOuterTypeParameter, "T").WithArguments("T", "C2").WithLocation(9, 28), + // (26,21): error CS0119: 'T()' is a method, which is not valid in the given context + // T.M(); + Diagnostic(ErrorCode.ERR_BadSKunknown, "T").WithArguments("T()", "method").WithLocation(26, 21), + // (23,23): warning CS0219: The variable 't' is assigned but its value is never used + // T t = default(T); + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "t").WithArguments("t").WithLocation(23, 23)); + } + [Fact] public void RefArgsInIteratorLocalFuncs() { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs index 6c75f3301f6..79729368a07 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests_Scope.cs @@ -15,6 +15,61 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests [CompilerTrait(CompilerFeature.Patterns)] public class PatternMatchingTests_Scope : PatternMatchingTestBase { + [Fact] + [WorkItem(13029, "https://github.com/dotnet/roslyn/issues/13029")] + public void ScopeOfLocalFunction() + { + var source = +@" +public class X +{ + public static void Main() + { + } + + bool Dummy(params object[] x) { return true; } + + void Test14(int val) + { + switch (val) + { + case 1 when TakeOutParam(true, out var x14): + void x14() {return;}; + break; + case 2: + x14(); + break; + } + + switch (val) + { + case 1 when Dummy(1 is var x14): + void x14() {return;}; + break; + case 2: + x14(); + break; + } + } + + static bool TakeOutParam(T y, out T x) + { + x = y; + return true; + } +} +"; + var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular); + + compilation.VerifyDiagnostics( + // (14,52): error CS0136: A local or parameter named 'x14' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // case 1 when TakeOutParam(true, out var x14): + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x14").WithArguments("x14").WithLocation(14, 52), + // (24,40): error CS0136: A local or parameter named 'x14' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // case 1 when Dummy(1 is var x14): + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x14").WithArguments("x14").WithLocation(24, 40)); + } + [Fact] public void ScopeOfPatternVariables_ExpressionStatement_01() { -- GitLab