Do not resolve diagnostics early during attribute binding

We were attempt to resolve diagnostics in ColorColor attribute argument scenarios while we were in the middle of attribute argument binding. This then caused us to need to bind attributes to resolve the diagnostic, entering an infinite loop that eventually overflows. To solve this, I've avoided binding diagnostics in this scenario by passing around a BindingDiagnosticBag, rather than an `ImmutableBindingDiagnostic<AssemblySymbol>`. Doing this does require some limited and controlled mutation in the tree; to help ensure future maintainability, the mutation only occurs in one direction (the binder taking ownership of the bag). However, if we're uncomfortable with the mutation approach, we can instead simply leak the BindingDiagnosticBags in this scenario. This is a small enough source of them that we're unlikely to see major issues not pooling them. Fixes dotnet#71039.

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

@@ -6846,7 +6846,7 @@ private BoundExpression BindLeftIdentifierOfPotentialColorColorMemberAccess(Iden
                                 // NOTE: ReplaceTypeOrValueReceiver will call CheckValue explicitly.
                                 boundValue = BindToNaturalType(boundValue, valueDiagnostics);
                                 return new BoundTypeOrValueExpression(left,
-                                    new BoundTypeOrValueData(leftSymbol, boundValue, valueDiagnostics.ToReadOnlyAndFree(), boundType, typeDiagnostics.ToReadOnlyAndFree()), leftType);
+                                    new BoundTypeOrValueData(leftSymbol, boundValue, valueDiagnostics, boundType, typeDiagnostics), leftType);
                             }
 
                             typeDiagnostics.Free();

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

@@ -1722,24 +1722,31 @@ private BoundExpression ReplaceTypeOrValueReceiver(BoundExpression receiver, boo
             {
                 case BoundKind.TypeOrValueExpression:
                     var typeOrValue = (BoundTypeOrValueExpression)receiver;
+                    typeOrValue.Data.TakeDiagnosticBagOwnership(out BindingDiagnosticBag? valueDiagnosticsBag, out BindingDiagnosticBag? typeDiagnosticsBag);
                     if (useType)
                     {
-                        diagnostics.AddRange(typeOrValue.Data.TypeDiagnostics);
+                        diagnostics.AddRange(typeDiagnosticsBag);
 
-                        foreach (Diagnostic d in typeOrValue.Data.ValueDiagnostics.Diagnostics)
+                        foreach (Diagnostic d in valueDiagnosticsBag.DiagnosticBag.AsEnumerableWithoutResolution())
                         {
-                            if (d.Code == (int)ErrorCode.WRN_PrimaryConstructorParameterIsShadowedAndNotPassedToBase &&
+                            if (d.IsCode((int)ErrorCode.WRN_PrimaryConstructorParameterIsShadowedAndNotPassedToBase) &&
                                 !(d.Arguments is [ParameterSymbol shadowedParameter] && shadowedParameter.Type.Equals(typeOrValue.Data.ValueExpression.Type, TypeCompareKind.AllIgnoreOptions))) // If the type and the name match, we would resolve to the same type rather than a value at the end.
                             {
                                 diagnostics.Add(d);
                             }
                         }
 
+                        valueDiagnosticsBag.Free();
+                        typeDiagnosticsBag.Free();
+
                         return typeOrValue.Data.TypeExpression;
                     }
                     else
                     {
-                        diagnostics.AddRange(typeOrValue.Data.ValueDiagnostics);
+                        diagnostics.AddRange(valueDiagnosticsBag);
+
+                        valueDiagnosticsBag.Free();
+                        typeDiagnosticsBag.Free();
                         return CheckValue(typeOrValue.Data.ValueExpression, BindValueKind.RValue, diagnostics);
                     }
 

src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs

@@ -636,59 +636,41 @@ public override bool SuppressVirtualCalls
     //       BoundTypeOrValueExpression from the bound tree generator, which would otherwise generate
     //       a constructor that may spuriously set hasErrors to true if either field had errors.
     //       A BoundTypeOrValueExpression should never have errors if it is present in the tree.
-    internal readonly struct BoundTypeOrValueData : System.IEquatable<BoundTypeOrValueData>
+    /// <remarks>
+    /// A BoundTypeOrValueData takes a BindingDiagnosticBag (including taking ownership of freeing that bag) instead of <see cref="ImmutableBindingDiagnostic{TAssemblySymbol}"/>
+    /// in order to avoid forcing resolution of lazy diagnostics. Early resolution can cause a binding cycle in color color tests involving assembly attribute arguments, so we
+    /// want to avoid that by delaying resolution until later in the process. The diagnostic bags are extracted by <see cref="Binder.ReplaceTypeOrValueReceiver(BoundExpression, bool, BindingDiagnosticBag)"/>,
+    /// which takes ownership of the diagnostic bags and frees them after extracting the diagnostics.
+    /// </remarks>
+    internal class BoundTypeOrValueData
     {
+        private Optional<BindingDiagnosticBag> _valueDiagnostics;
+        private Optional<BindingDiagnosticBag> _typeDiagnostics;
+
         public Symbol ValueSymbol { get; }
         public BoundExpression ValueExpression { get; }
-        public ImmutableBindingDiagnostic<AssemblySymbol> ValueDiagnostics { get; }
         public BoundExpression TypeExpression { get; }
-        public ImmutableBindingDiagnostic<AssemblySymbol> TypeDiagnostics { get; }
 
-        public BoundTypeOrValueData(Symbol valueSymbol, BoundExpression valueExpression, ImmutableBindingDiagnostic<AssemblySymbol> valueDiagnostics, BoundExpression typeExpression, ImmutableBindingDiagnostic<AssemblySymbol> typeDiagnostics)
+        public BoundTypeOrValueData(Symbol valueSymbol, BoundExpression valueExpression, BindingDiagnosticBag valueDiagnostics, BoundExpression typeExpression, BindingDiagnosticBag typeDiagnostics)
         {
             Debug.Assert(valueSymbol != null, "Field 'valueSymbol' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
             Debug.Assert(valueExpression != null, "Field 'valueExpression' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
             Debug.Assert(typeExpression != null, "Field 'typeExpression' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
 
             this.ValueSymbol = valueSymbol;
             this.ValueExpression = valueExpression;
-            this.ValueDiagnostics = valueDiagnostics;
+            _valueDiagnostics = valueDiagnostics;
             this.TypeExpression = typeExpression;
-            this.TypeDiagnostics = typeDiagnostics;
-        }
-
-        // operator==, operator!=, GetHashCode, and Equals are needed by the generated bound tree.
-
-        public static bool operator ==(BoundTypeOrValueData a, BoundTypeOrValueData b)
-        {
-            return (object)a.ValueSymbol == (object)b.ValueSymbol &&
-                (object)a.ValueExpression == (object)b.ValueExpression &&
-                a.ValueDiagnostics == b.ValueDiagnostics &&
-                (object)a.TypeExpression == (object)b.TypeExpression &&
-                a.TypeDiagnostics == b.TypeDiagnostics;
-        }
-
-        public static bool operator !=(BoundTypeOrValueData a, BoundTypeOrValueData b)
-        {
-            return !(a == b);
-        }
-
-        public override bool Equals(object? obj)
-        {
-            return obj is BoundTypeOrValueData && (BoundTypeOrValueData)obj == this;
-        }
-
-        public override int GetHashCode()
-        {
-            return Hash.Combine(ValueSymbol.GetHashCode(),
-                Hash.Combine(ValueExpression.GetHashCode(),
-                Hash.Combine(ValueDiagnostics.GetHashCode(),
-                Hash.Combine(TypeExpression.GetHashCode(), TypeDiagnostics.GetHashCode()))));
+            _typeDiagnostics = typeDiagnostics;
         }
 
-        bool System.IEquatable<BoundTypeOrValueData>.Equals(BoundTypeOrValueData b)
+        public void TakeDiagnosticBagOwnership(out BindingDiagnosticBag valueDiagnostics, out BindingDiagnosticBag typeDiagnostics)
         {
-            return b == this;
+            Debug.Assert(_typeDiagnostics.HasValue && _valueDiagnostics.HasValue);
+            valueDiagnostics = _valueDiagnostics.Value;
+            typeDiagnostics = _typeDiagnostics.Value;
+            _valueDiagnostics = default;
+            _typeDiagnostics = default;
         }
     }
 

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -30,7 +30,6 @@
   <ValueType Name="LookupResultKind"/>
   <ValueType Name="NoOpStatementFlavor"/>
   <ValueType Name="RefKind"/>
-  <ValueType Name="BoundTypeOrValueData"/>
   <ValueType Name="BoundLocalDeclarationKind"/>
   <ValueType Name="NullableAnnotation"/>
   <ValueType Name="ErrorCode"/>

src/Compilers/CSharp/Test/Semantic/Semantics/ColorColorTests.cs

@@ -2366,5 +2366,64 @@ instance void .ctor () cil managed
 
             compilation.VerifyDiagnostics();
         }
+
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71039")]
+        public void ConstInAttributes_NoCycle_01()
+        {
+            var source = """
+                using ILGPU;
+                using System.Runtime.CompilerServices;
+
+                [assembly: InternalsVisibleTo(Context.RuntimeAssemblyName)]
+
+                namespace ILGPU
+                {
+                    public class Context
+                    {
+                        public const string RuntimeAssemblyName = RuntimeSystem.AssemblyName;
+                        public RuntimeSystem RuntimeSystem { get; }
+                    }
+                }
+                """;
+
+            CreateCompilation(source).VerifyDiagnostics(
+                // (4,31): error CS0182: An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type
+                // [assembly: InternalsVisibleTo(Context.RuntimeAssemblyName)]
+                Diagnostic(ErrorCode.ERR_BadAttributeArgument, "Context.RuntimeAssemblyName").WithLocation(4, 31),
+                // (10,65): error CS1061: 'RuntimeSystem' does not contain a definition for 'AssemblyName' and no accessible extension method 'AssemblyName' accepting a first argument of type 'RuntimeSystem' could be found (are you missing a using directive or an assembly reference?)
+                //         public const string RuntimeAssemblyName = RuntimeSystem.AssemblyName;
+                Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "AssemblyName").WithArguments("RuntimeSystem", "AssemblyName").WithLocation(10, 65),
+                // (11,16): error CS0246: The type or namespace name 'RuntimeSystem' could not be found (are you missing a using directive or an assembly reference?)
+                //         public RuntimeSystem RuntimeSystem { get; }
+                Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "RuntimeSystem").WithArguments("RuntimeSystem").WithLocation(11, 16)
+            );
+        }
+
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71039")]
+        public void ConstInAttributes_NoCycle_02()
+        {
+            var source = """
+                using ILGPU;
+                using System.Runtime.CompilerServices;
+
+                [assembly: InternalsVisibleTo(Context.RuntimeAssemblyName)]
+
+                namespace ILGPU
+                {
+                    public class Context
+                    {
+                        public const string RuntimeAssemblyName = RuntimeSystem.AssemblyName;
+                        public RuntimeSystem RuntimeSystem { get; }
+                    }
+
+                    public class RuntimeSystem
+                    {
+                        public const string AssemblyName = "RuntimeSystem";
+                    }
+                }
+                """;
+
+            CompileAndVerify(source).VerifyDiagnostics();
+        }
     }
 }

src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs

@@ -476,6 +476,11 @@ internal Diagnostic WithProgrammaticSuppression(ProgrammaticSuppressionInfo prog
         // compatibility
         internal virtual int Code { get { return 0; } }
 
+        /// <summary>
+        /// True if this diagnostic represents the given error code. This does not force resolution of lazy diagnostics and is safe to call at any time.
+        /// </summary>
+        internal virtual bool IsCode(int code) { return false; }
+
         internal virtual IReadOnlyList<object?> Arguments
         {
             get { return SpecializedCollections.EmptyReadOnlyList<object?>(); }

src/Compilers/Core/Portable/Diagnostic/DiagnosticWithInfo.cs

@@ -147,6 +147,14 @@ internal DiagnosticInfo LazyInfo
             }
         }
 
+        /// <summary>
+        /// True if this diagnostic represents the given error code. This does not force resolution of lazy diagnostics and is safe to call at any time.
+        /// </summary>
+        internal override bool IsCode(int code)
+        {
+            return _info.Code == code;
+        }
+
         public override int GetHashCode()
         {
             return Hash.Combine(this.Location.GetHashCode(), this.Info.GetHashCode());

src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilerDiagnosticAnalyzer.CompilationAnalyzer.cs

@@ -89,6 +89,7 @@ public CompilerDiagnostic(Diagnostic original, ImmutableDictionary<string, strin
 #pragma warning restore RS0013 
 
                 internal override int Code => _original.Code;
+                internal override bool IsCode(int code) => _original.IsCode(code);
                 internal override IReadOnlyList<object?> Arguments => _original.Arguments;
 
                 public override string Id => _original.Id;