Fix stack overflow compiling deeply nested generic

As a part of implementing nullable reference types many of our locals
switched from `TypeSymbol` to `TypeSymbolWithAnnotations`. In the vast
majority of cases this doesn't have a meaningful impact on compilation.
They are bigger (about 3X) but it's still a relatively small `struct`
(three words).

The size difference is significant though in
`BindNamespaceOrTypeOrAliasSymbol`. This method is used in recursive
parts of binding and is mutually recursive with `BindQualifiedNam`. This
method defines a large number of locals which contribute to every layer
of recursion. When they moved to `TypeSymbolWithAnnotations` this pushed
us outside our tolerance levels and we hit an overflow in extreme cases.

Virtually none of these locals are used in the recursive case. Factored
their use into local functions so we only pay the stack usage on demand.

closes #33909
fixes https://github.com/dotnet/coreclr/issues/22757

src/Compilers/CSharp/Portable/Binder/Binder.NamespaceOrTypeOrAliasSymbolWithAnnotations.cs

@@ -7,7 +7,7 @@ namespace Microsoft.CodeAnalysis.CSharp
 {
     internal partial class Binder
     {
-        internal struct NamespaceOrTypeOrAliasSymbolWithAnnotations
+        internal readonly struct NamespaceOrTypeOrAliasSymbolWithAnnotations
         {
             private readonly TypeSymbolWithAnnotations _type;
             private readonly Symbol _symbol;

src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs

@@ -5,6 +5,7 @@
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.PooledObjects;
@@ -330,6 +331,11 @@ internal NamespaceOrTypeOrAliasSymbolWithAnnotations BindNamespaceOrTypeSymbol(E
             return BindNamespaceOrTypeSymbol(syntax, diagnostics, basesBeingResolved, basesBeingResolved != null);
         }
 
+        /// <summary>
+        /// This method is used in deeply recursive parts of the compiler and requires a non-trivial amount of stack
+        /// space to execute. Preventing inlining here to keep recursive frames small.
+        /// </summary>
+        [MethodImpl(MethodImplOptions.NoInlining)]
         internal NamespaceOrTypeOrAliasSymbolWithAnnotations BindNamespaceOrTypeSymbol(ExpressionSyntax syntax, DiagnosticBag diagnostics, ConsList<TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics)
         {
             var result = BindNamespaceOrTypeOrAliasSymbol(syntax, diagnostics, basesBeingResolved, suppressUseSiteDiagnostics);
@@ -338,44 +344,28 @@ internal NamespaceOrTypeOrAliasSymbolWithAnnotations BindNamespaceOrTypeSymbol(E
             return UnwrapAlias(result, diagnostics, syntax, basesBeingResolved);
         }
 
+        /// <summary>
+        /// Bind the syntax into a namespace, type or alias symbol. 
+        /// </summary>
+        /// <remarks>
+        /// This method is used in deeply recursive parts of the compiler. Specifically this and <see cref="BindQualifiedName(ExpressionSyntax, SimpleNameSyntax, DiagnosticBag, ConsList{TypeSymbol}, bool)"/>
+        /// are mutually recursive. The non-recursive parts of this method tend to reserve siginficantly large 
+        /// stack frames due to their use of large struct like <see cref="TypeSymbolWithAnnotations"/>.
+        ///
+        /// To keep the stack frame size on recursive paths small the non-recursive parts are factored into local 
+        /// functions. This means we pay their stack penalty only when they are used. They are themselves big 
+        /// enough they should be disqualified from inlining. In the future when attributes are allowed on 
+        /// local functions we should explicitly mark them as <see cref="MethodImplOptions.NoInlining"/>
+        /// </remarks>
         internal NamespaceOrTypeOrAliasSymbolWithAnnotations BindNamespaceOrTypeOrAliasSymbol(ExpressionSyntax syntax, DiagnosticBag diagnostics, ConsList<TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics)
         {
             switch (syntax.Kind())
             {
                 case SyntaxKind.NullableType:
-                    {
-                        var nullableSyntax = (NullableTypeSyntax)syntax;
-                        TypeSyntax typeArgumentSyntax = nullableSyntax.ElementType;
-                        TypeSymbolWithAnnotations typeArgument = BindType(typeArgumentSyntax, diagnostics, basesBeingResolved);
-                        TypeSymbolWithAnnotations constructedType = typeArgument.SetIsAnnotated(Compilation);
-
-                        reportNullableReferenceTypesIfNeeded(nullableSyntax.QuestionToken, typeArgument);
-
-                        if (!ShouldCheckConstraints)
-                        {
-                            diagnostics.Add(new LazyUseSiteDiagnosticsInfoForNullableType(constructedType), syntax.GetLocation());
-                        }
-                        else if (constructedType.IsNullableType())
-                        {
-                            ReportUseSiteDiagnostics(constructedType.TypeSymbol.OriginalDefinition, diagnostics, syntax);
-                            var type = (NamedTypeSymbol)constructedType.TypeSymbol;
-                            var location = syntax.Location;
-                            type.CheckConstraints(this.Compilation, this.Conversions, includeNullability: true, location, diagnostics);
-                        }
-                        else if (constructedType.TypeSymbol.IsTypeParameterDisallowingAnnotation())
-                        {
-                            diagnostics.Add(ErrorCode.ERR_NullableUnconstrainedTypeParameter, syntax.Location);
-                        }
-
-                        return constructedType;
-                    }
+                    return bindNullable();
 
                 case SyntaxKind.PredefinedType:
-                    {
-                        var predefinedType = (PredefinedTypeSyntax)syntax;
-                        var type = BindPredefinedTypeSymbol(predefinedType, diagnostics);
-                        return TypeSymbolWithAnnotations.Create(IsNullableEnabled(predefinedType.Keyword), type);
-                    }
+                    return bindPredefined();
 
                 case SyntaxKind.IdentifierName:
                     return BindNonGenericSimpleNamespaceOrTypeOrAliasSymbol((IdentifierNameSyntax)syntax, diagnostics, basesBeingResolved, suppressUseSiteDiagnostics, qualifierOpt: null);
@@ -384,19 +374,7 @@ internal NamespaceOrTypeOrAliasSymbolWithAnnotations BindNamespaceOrTypeOrAliasS
                     return BindGenericSimpleNamespaceOrTypeOrAliasSymbol((GenericNameSyntax)syntax, diagnostics, basesBeingResolved, qualifierOpt: null);
 
                 case SyntaxKind.AliasQualifiedName:
-                    {
-                        var node = (AliasQualifiedNameSyntax)syntax;
-                        var bindingResult = BindNamespaceAliasSymbol(node.Alias, diagnostics);
-                        var alias = bindingResult as AliasSymbol;
-                        NamespaceOrTypeSymbol left = ((object)alias != null) ? alias.Target : (NamespaceOrTypeSymbol)bindingResult;
-
-                        if (left.Kind == SymbolKind.NamedType)
-                        {
-                            return TypeSymbolWithAnnotations.Create(new ExtendedErrorTypeSymbol(left, LookupResultKind.NotATypeOrNamespace, diagnostics.Add(ErrorCode.ERR_ColColWithTypeAlias, node.Alias.Location, node.Alias.Identifier.Text)));
-                        }
-
-                        return this.BindSimpleNamespaceOrTypeOrAliasSymbol(node.Name, diagnostics, basesBeingResolved, suppressUseSiteDiagnostics, left);
-                    }
+                    return bindAlias();
 
                 case SyntaxKind.QualifiedName:
                     {
@@ -416,24 +394,7 @@ internal NamespaceOrTypeOrAliasSymbolWithAnnotations BindNamespaceOrTypeOrAliasS
                     }
 
                 case SyntaxKind.PointerType:
-                    {
-                        var node = (PointerTypeSyntax)syntax;
-                        var elementType = BindType(node.ElementType, diagnostics, basesBeingResolved);
-                        ReportUnsafeIfNotAllowed(node, diagnostics);
-
-                        // Checking BinderFlags.GenericConstraintsClause to prevent cycles in binding
-                        if (Flags.HasFlag(BinderFlags.GenericConstraintsClause) && elementType.TypeKind == TypeKind.TypeParameter)
-                        {
-                            // Invalid constraint type. A type used as a constraint must be an interface, a non-sealed class or a type parameter.
-                            Error(diagnostics, ErrorCode.ERR_BadConstraintType, node);
-                        }
-                        else
-                        {
-                            CheckManagedAddr(elementType.TypeSymbol, node, diagnostics);
-                        }
-
-                        return TypeSymbolWithAnnotations.Create(new PointerTypeSymbol(elementType));
-                    }
+                    return bindPointer();
 
                 case SyntaxKind.OmittedTypeArgument:
                     {
@@ -478,6 +439,76 @@ void reportNullableReferenceTypesIfNeeded(SyntaxToken questionToken, TypeSymbolW
                     LazyMissingNonNullTypesContextDiagnosticInfo.ReportNullableReferenceTypesIfNeeded(isNullableEnabled, typeArgument, location, diagnostics);
                 }
             }
+
+            NamespaceOrTypeOrAliasSymbolWithAnnotations bindNullable()
+            {
+                var nullableSyntax = (NullableTypeSyntax)syntax;
+                TypeSyntax typeArgumentSyntax = nullableSyntax.ElementType;
+                TypeSymbolWithAnnotations typeArgument = BindType(typeArgumentSyntax, diagnostics, basesBeingResolved);
+                TypeSymbolWithAnnotations constructedType = typeArgument.SetIsAnnotated(Compilation);
+
+                reportNullableReferenceTypesIfNeeded(nullableSyntax.QuestionToken, typeArgument);
+
+                if (!ShouldCheckConstraints)
+                {
+                    diagnostics.Add(new LazyUseSiteDiagnosticsInfoForNullableType(constructedType), syntax.GetLocation());
+                }
+                else if (constructedType.IsNullableType())
+                {
+                    ReportUseSiteDiagnostics(constructedType.TypeSymbol.OriginalDefinition, diagnostics, syntax);
+                    var type = (NamedTypeSymbol)constructedType.TypeSymbol;
+                    var location = syntax.Location;
+                    type.CheckConstraints(this.Compilation, this.Conversions, includeNullability: true, location, diagnostics);
+                }
+                else if (constructedType.TypeSymbol.IsTypeParameterDisallowingAnnotation())
+                {
+                    diagnostics.Add(ErrorCode.ERR_NullableUnconstrainedTypeParameter, syntax.Location);
+                }
+
+                return constructedType;
+            }
+
+            NamespaceOrTypeOrAliasSymbolWithAnnotations bindPredefined()
+            {
+                var predefinedType = (PredefinedTypeSyntax)syntax;
+                var type = BindPredefinedTypeSymbol(predefinedType, diagnostics);
+                return TypeSymbolWithAnnotations.Create(IsNullableEnabled(predefinedType.Keyword), type);
+            }
+
+            NamespaceOrTypeOrAliasSymbolWithAnnotations bindAlias()
+            {
+                var node = (AliasQualifiedNameSyntax)syntax;
+                var bindingResult = BindNamespaceAliasSymbol(node.Alias, diagnostics);
+                var alias = bindingResult as AliasSymbol;
+                NamespaceOrTypeSymbol left = ((object)alias != null) ? alias.Target : (NamespaceOrTypeSymbol)bindingResult;
+
+                if (left.Kind == SymbolKind.NamedType)
+                {
+                    return TypeSymbolWithAnnotations.Create(new ExtendedErrorTypeSymbol(left, LookupResultKind.NotATypeOrNamespace, diagnostics.Add(ErrorCode.ERR_ColColWithTypeAlias, node.Alias.Location, node.Alias.Identifier.Text)));
+                }
+
+                return this.BindSimpleNamespaceOrTypeOrAliasSymbol(node.Name, diagnostics, basesBeingResolved, suppressUseSiteDiagnostics, left);
+            }
+
+            NamespaceOrTypeOrAliasSymbolWithAnnotations bindPointer()
+            {
+                var node = (PointerTypeSyntax)syntax;
+                var elementType = BindType(node.ElementType, diagnostics, basesBeingResolved);
+                ReportUnsafeIfNotAllowed(node, diagnostics);
+
+                // Checking BinderFlags.GenericConstraintsClause to prevent cycles in binding
+                if (Flags.HasFlag(BinderFlags.GenericConstraintsClause) && elementType.TypeKind == TypeKind.TypeParameter)
+                {
+                    // Invalid constraint type. A type used as a constraint must be an interface, a non-sealed class or a type parameter.
+                    Error(diagnostics, ErrorCode.ERR_BadConstraintType, node);
+                }
+                else
+                {
+                    CheckManagedAddr(elementType.TypeSymbol, node, diagnostics);
+                }
+
+                return TypeSymbolWithAnnotations.Create(new PointerTypeSymbol(elementType));
+            }
         }
 
         private TypeSymbolWithAnnotations BindArrayType(

src/Compilers/CSharp/Test/Emit/Emit/EndToEndTests.cs

@@ -6,10 +6,28 @@
 using System.Threading.Tasks;
 using Xunit;
 using Microsoft.CodeAnalysis.Test.Utilities;
+using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
+
 namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Emit
 {
     public class EndToEndTests : EmitMetadataTestBase
     {
+        /// <summary>
+        /// These tests are very sensitive to stack size hence we use a fresh thread to ensure there 
+        /// is a consistent stack size for them to execute in. 
+        /// </summary>
+        /// <param name="action"></param>
+        private static void RunInThread(Action action)
+        {
+            var thread = new System.Threading.Thread(() =>
+            {
+                action();
+            }, 0);
+            thread.Start();
+            thread.Join();
+        }
+
+
         // This test is a canary attempting to make sure that we don't regress the # of fluent calls that 
         // the compiler can handle.
         [WorkItem(16669, "https://github.com/dotnet/roslyn/issues/16669")]
@@ -75,16 +93,68 @@ void tryCompileDeepFluentCalls(int depth)
 }");
 
                 var source = builder.ToString();
-
-                var thread = new System.Threading.Thread(() =>
+                RunInThread(() =>
                 {
                     var options = new CSharpCompilationOptions(outputKind: OutputKind.DynamicallyLinkedLibrary, concurrentBuild: false);
                     var compilation = CreateCompilation(source, options: options);
                     compilation.VerifyDiagnostics();
                     compilation.EmitToArray();
-                }, 0);
-                thread.Start();
-                thread.Join();
+                });
+            }
+        }
+
+        [Fact]
+        [WorkItem(33909, "https://github.com/dotnet/roslyn/issues/33909")]
+        public void DeeplyNestedGeneric()
+        {
+            int nestingLevel = 500;
+            var builder = new StringBuilder();
+            builder.AppendLine(@"
+using System;
+
+public class Test
+{
+    public static int Main()
+    {
+        #pragma warning disable 219
+");
+
+            for (var i = 0; i < nestingLevel; i++)
+            {
+                if (i > 0)
+                {
+                    builder.Append('.');
+                }
+                builder.Append($"MyStruct{i}<int>");
+            }
+
+            builder.AppendLine(" local;");
+            builder.AppendLine(@"
+        #pragma warning restore 219
+        Console.WriteLine(""Pass"");
+    }
+}");
+
+            genType(0);
+
+            var source = builder.ToString();
+
+            RunInThread(() =>
+            {
+                var compilation = CreateCompilation(source, options: TestOptions.DebugExe);
+                CompileAndVerify(compilation, expectedOutput: "Pass");
+            });
+
+            void genType(int level)
+            {
+                if (level >= nestingLevel)
+                {
+                    return;
+                }
+
+                builder.AppendLine($"public struct MyStruct{level}<T{level}> {{");
+                genType(level + 1);
+                builder.AppendLine("}");
             }
         }
     }