Fix issue 24978 (circular field definition)

jcouv · Apr 27, 2018 · 0ada9b3 · 0ada9b3
1 parent bab0f7e
commit 0ada9b3
Show file tree

Hide file tree

Showing 35 changed files with 203 additions and 125 deletions.
diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
@@ -1120,7 +1120,7 @@ private BoundExpression BindSizeOf(SizeOfExpressionSyntax node, DiagnosticBag di
 
             bool typeHasErrors = type.IsErrorType();
 
-            if (!typeHasErrors && type.IsManagedType)
+            if (!typeHasErrors && type.IsManagedType(fieldsBeingBound: null))
             {
                 diagnostics.Add(ErrorCode.ERR_ManagedAddr, node.Location, type);
                 typeHasErrors = true;
@@ -2717,7 +2717,7 @@ private BoundExpression BindImplicitStackAllocArrayCreationExpression(ImplicitSt
                 bestType = CreateErrorType();
             }
 
-            if (!bestType.IsErrorType() && bestType.IsManagedType)
+            if (!bestType.IsErrorType() && bestType.IsManagedType(fieldsBeingBound: null))
             {
                 Error(diagnostics, ErrorCode.ERR_ManagedAddr, node, bestType);
             }
@@ -3070,7 +3070,7 @@ private BoundExpression BindStackAllocArrayCreationExpression(
             TypeSymbol elementType = BindType(elementTypeSyntax, diagnostics);
 
             TypeSymbol type = GetStackAllocType(node, elementType, diagnostics, out bool hasErrors);
-            if (!elementType.IsErrorType() && elementType.IsManagedType)
+            if (!elementType.IsErrorType() && elementType.IsManagedType(fieldsBeingBound: null))
             {
                 Error(diagnostics, ErrorCode.ERR_ManagedAddr, elementTypeSyntax, elementType);
                 hasErrors = true;

diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
@@ -2107,7 +2107,7 @@ private BoundExpression BindAddressOfExpression(PrefixUnaryExpressionSyntax node
             TypeSymbol operandType = operand.Type;
             Debug.Assert((object)operandType != null, "BindValue should have caught a null operand type");
 
-            bool isManagedType = operandType.IsManagedType;
+            bool isManagedType = operandType.IsManagedType(fieldsBeingBound: null);
             bool allowManagedAddressOf = Flags.Includes(BinderFlags.AllowManagedAddressOf);
             if (!allowManagedAddressOf)
             {

diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
@@ -1139,7 +1139,7 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym
                     }
             }
 
-            if (elementType.IsManagedType)
+            if (elementType.IsManagedType(fieldsBeingBound: null))
             {
                 Error(diagnostics, ErrorCode.ERR_ManagedAddr, initializerSyntax, elementType);
                 hasErrors = true;

diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
@@ -232,9 +232,9 @@ private Symbol BindTypeOrAliasOrKeyword(TypeSyntax syntax, DiagnosticBag diagnos
         // Binds the given expression syntax as Type.
         // If the resulting symbol is an Alias to a Type, it unwraps the alias
         // and returns it's target type.
-        internal TypeSymbol BindType(ExpressionSyntax syntax, DiagnosticBag diagnostics, ConsList<Symbol> basesBeingResolved = null)
+        internal TypeSymbol BindType(ExpressionSyntax syntax, DiagnosticBag diagnostics, ConsList<Symbol> basesBeingResolved = null, ConsList<FieldSymbol> fieldsBeingBound = null)
         {
-            var symbol = BindTypeOrAlias(syntax, diagnostics, basesBeingResolved);
+            var symbol = BindTypeOrAlias(syntax, diagnostics, basesBeingResolved, fieldsBeingBound);
             return (TypeSymbol)UnwrapAlias(symbol, diagnostics, syntax, basesBeingResolved);
         }
 
@@ -250,11 +250,11 @@ internal TypeSymbol BindType(ExpressionSyntax syntax, DiagnosticBag diagnostics,
         // Binds the given expression syntax as Type or an Alias to Type
         // and returns the resultant symbol.
         // NOTE: This method doesn't unwrap aliases.
-        internal Symbol BindTypeOrAlias(ExpressionSyntax syntax, DiagnosticBag diagnostics, ConsList<Symbol> basesBeingResolved = null)
+        internal Symbol BindTypeOrAlias(ExpressionSyntax syntax, DiagnosticBag diagnostics, ConsList<Symbol> basesBeingResolved = null, ConsList<FieldSymbol> fieldsBeingBound = null)
         {
             Debug.Assert(diagnostics != null);
 
-            var symbol = BindNamespaceOrTypeOrAliasSymbol(syntax, diagnostics, basesBeingResolved, basesBeingResolved != null);
+            var symbol = BindNamespaceOrTypeOrAliasSymbol(syntax, diagnostics, basesBeingResolved, basesBeingResolved != null, fieldsBeingBound);
 
             // symbol must be a TypeSymbol or an Alias to a TypeSymbol
             var result = UnwrapAliasNoDiagnostics(symbol, basesBeingResolved) as TypeSymbol;
@@ -318,7 +318,8 @@ internal NamespaceOrTypeSymbol BindNamespaceOrTypeSymbol(ExpressionSyntax syntax
             return (NamespaceOrTypeSymbol)UnwrapAlias(result, diagnostics, syntax, basesBeingResolved);
         }
 
-        internal Symbol BindNamespaceOrTypeOrAliasSymbol(ExpressionSyntax syntax, DiagnosticBag diagnostics, ConsList<Symbol> basesBeingResolved, bool suppressUseSiteDiagnostics)
+        internal Symbol BindNamespaceOrTypeOrAliasSymbol(ExpressionSyntax syntax, DiagnosticBag diagnostics, ConsList<Symbol> basesBeingResolved,
+            bool suppressUseSiteDiagnostics, ConsList<FieldSymbol> fieldsBeingBound = null)
         {
             switch (syntax.Kind())
             {
@@ -375,7 +376,7 @@ internal Symbol BindNamespaceOrTypeOrAliasSymbol(ExpressionSyntax syntax, Diagno
                 case SyntaxKind.ArrayType:
                     {
                         var node = (ArrayTypeSyntax)syntax;
-                        TypeSymbol type = BindType(node.ElementType, diagnostics, basesBeingResolved);
+                        TypeSymbol type = BindType(node.ElementType, diagnostics, basesBeingResolved, fieldsBeingBound);
                         if (type.IsStatic)
                         {
                             // CS0719: '{0}': array elements cannot be of static type
@@ -409,7 +410,7 @@ internal Symbol BindNamespaceOrTypeOrAliasSymbol(ExpressionSyntax syntax, Diagno
                             // 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 if (elementType.IsManagedType)
+                        else if (elementType.IsManagedType(fieldsBeingBound))
                         {
                             // "Cannot take the address of, get the size of, or declare a pointer to a managed type ('{0}')"
                             Error(diagnostics, ErrorCode.ERR_ManagedAddr, node, elementType);

diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs
diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx
@@ -720,6 +720,9 @@
   <data name="ERR_CircularBase" xml:space="preserve">
     <value>Circular base class dependency involving '{0}' and '{1}'</value>
   </data>
+  <data name="ERR_CircularField" xml:space="preserve">
+    <value>The definition of field '{0}' is circular</value>
+  </data>
   <data name="ERR_BadDelegateConstructor" xml:space="preserve">
     <value>The delegate '{0}' does not have a valid constructor</value>
   </data>

diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
@@ -1577,6 +1577,8 @@ internal enum ErrorCode
         ERR_ExprCannotBeFixed = 8385,
         ERR_InvalidObjectCreation = 8386,
         #endregion diagnostics introduced for C# 7.3
+
+        ERR_CircularField = 8387,
     }
     // Note: you will need to re-generate compiler code after adding warnings (build\scripts\generate-compiler-code.cmd)
 }
diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs
@@ -40,7 +40,7 @@ public override BoundNode VisitFixedStatement(BoundFixedStatement node)
                 }
                 else
                 {
-                    Debug.Assert(!pinnedTemp.Type.IsManagedType);
+                    Debug.Assert(!pinnedTemp.Type.IsManagedType(fieldsBeingBound: null));
 
                     // temp = ref *default(T*);
                     cleanup[i] = _factory.Assignment(_factory.Local(pinnedTemp), new BoundPointerIndirectionOperator(

diff --git a/src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs
@@ -234,12 +234,9 @@ public override bool IsValueType
             }
         }
 
-        internal sealed override bool IsManagedType
+        internal sealed override bool IsManagedType(ConsList<FieldSymbol> fieldsBeingBound)
         {
-            get
-            {
-                return true;
-            }
+            return true;
         }
 
         internal sealed override bool IsByRefLikeType

diff --git a/src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs b/src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs
@@ -106,7 +106,7 @@ private static void StructDependsClosure(NamedTypeSymbol type, HashSet<Symbol> p
         /// be managed even if it had no fields.  e.g. struct S { S s; } is not managed, but struct S { S s; object o; }
         /// is because we can point to object.
         /// </summary>
-        internal static bool IsManagedType(NamedTypeSymbol type)
+        internal static bool IsManagedType(NamedTypeSymbol type, ConsList<FieldSymbol> fieldsBeingBound = null)
         {
             // If this is a type with an obvious answer, return quickly.
             switch (IsManagedTypeHelper(type))
@@ -119,12 +119,12 @@ internal static bool IsManagedType(NamedTypeSymbol type)
 
             // Otherwise, we have to build and inspect the closure of depended-upon types.
             var hs = PooledHashSet<Symbol>.GetInstance();
-            bool result = DependsOnDefinitelyManagedType(type, hs);
+            bool result = DependsOnDefinitelyManagedType(type, hs, fieldsBeingBound);
             hs.Free();
             return result;
         }
 
-        private static bool DependsOnDefinitelyManagedType(NamedTypeSymbol type, HashSet<Symbol> partialClosure)
+        private static bool DependsOnDefinitelyManagedType(NamedTypeSymbol type, HashSet<Symbol> partialClosure, ConsList<FieldSymbol> fieldsBeingBound = null)
         {
             Debug.Assert((object)type != null);
 
@@ -164,11 +164,11 @@ private static bool DependsOnDefinitelyManagedType(NamedTypeSymbol type, HashSet
                         continue;
                     }
 
-                    TypeSymbol fieldType = field.Type;
+                    TypeSymbol fieldType = field.GetFieldType(fieldsBeingBound);
                     NamedTypeSymbol fieldNamedType = fieldType as NamedTypeSymbol;
                     if ((object)fieldNamedType == null)
                     {
-                        if (fieldType.IsManagedType)
+                        if (fieldType.IsManagedType(fieldsBeingBound: null))
                         {
                             return true;
                         }

diff --git a/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs b/src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs
@@ -817,7 +817,7 @@ private static bool CheckConstraints(
                 return false;
             }
 
-            if (typeParameter.HasUnmanagedTypeConstraint && (typeArgument.IsManagedType || !typeArgument.IsNonNullableValueType()))
+            if (typeParameter.HasUnmanagedTypeConstraint && (typeArgument.IsManagedType(fieldsBeingBound: null) || !typeArgument.IsNonNullableValueType()))
             {
                 // "The type '{2}' must be a non-nullable value type, along with all fields at any level of nesting, in order to use it as parameter '{1}' in the generic type or method '{0}'"
                 diagnosticsBuilder.Add(new TypeParameterDiagnosticInfo(typeParameter, new CSDiagnosticInfo(ErrorCode.ERR_UnmanagedConstraintNotSatisfied, containingSymbol.ConstructedFrom(), typeParameter, typeArgument)));

diff --git a/src/Compilers/CSharp/Portable/Symbols/DynamicTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/DynamicTypeSymbol.cs
@@ -107,12 +107,9 @@ public override bool IsValueType
             }
         }
 
-        internal sealed override bool IsManagedType
+        internal sealed override bool IsManagedType(ConsList<FieldSymbol> fieldsBeingBound)
         {
-            get
-            {
-                return true;
-            }
+            return true;
         }
 
         internal sealed override bool IsByRefLikeType

diff --git a/src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs
@@ -414,15 +414,12 @@ public override bool IsValueType
             }
         }
 
-        internal override bool IsManagedType
+        internal override bool IsManagedType(ConsList<FieldSymbol> fieldsBeingBound = null)
         {
-            get
-            {
-                // CONSIDER: we could cache this, but it's only expensive for non-special struct types
-                // that are pointed to.  For now, only cache on SourceMemberContainerSymbol since it fits
-                // nicely into the flags variable.
-                return BaseTypeAnalysis.IsManagedType(this);
-            }
+            // CONSIDER: we could cache this, but it's only expensive for non-special struct types
+            // that are pointed to.  For now, only cache on SourceMemberContainerSymbol since it fits
+            // nicely into the flags variable.
+            return BaseTypeAnalysis.IsManagedType(this, fieldsBeingBound);
         }
 
         /// <summary>

diff --git a/src/Compilers/CSharp/Portable/Symbols/PointerTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/PointerTypeSymbol.cs
@@ -120,12 +120,9 @@ public override bool IsValueType
             }
         }
 
-        internal sealed override bool IsManagedType
+        internal sealed override bool IsManagedType(ConsList<FieldSymbol> fieldsBeingBound)
         {
-            get
-            {
-                return false;
-            }
+            return false;
         }
 
         internal sealed override bool IsByRefLikeType

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs
@@ -682,19 +682,16 @@ internal sealed override bool IsInterface
             }
         }
 
-        internal override bool IsManagedType
+        internal override bool IsManagedType(ConsList<FieldSymbol> fieldsBeingBound)
         {
-            get
+            var isManagedType = _flags.IsManagedType;
+            if (!isManagedType.HasValue())
             {
-                var isManagedType = _flags.IsManagedType;
-                if (!isManagedType.HasValue())
-                {
-                    bool value = base.IsManagedType;
-                    _flags.SetIsManagedType(value);
-                    return value;
-                }
-                return isManagedType.Value();
+                bool value = base.IsManagedType(fieldsBeingBound);
+                _flags.SetIsManagedType(value);
+                return value;
             }
+            return isManagedType.Value();
         }
 
         public override bool IsStatic

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs
@@ -401,8 +401,8 @@ internal sealed override TypeSymbol GetFieldType(ConsList<FieldSymbol> fieldsBei
             var typeSyntax = fieldSyntax.Declaration.Type;
 
             var compilation = this.DeclaringCompilation;
-
             var diagnostics = DiagnosticBag.GetInstance();
+
             TypeSymbol type;
 
             // When we have multiple declarators, we report the type diagnostics on only the first.
@@ -434,7 +434,15 @@ internal sealed override TypeSymbol GetFieldType(ConsList<FieldSymbol> fieldsBei
                 binder = binder.WithContainingMemberOrLambda(this);
                 if (!ContainingType.IsScriptClass)
                 {
-                    type = binder.BindType(typeSyntax, diagnosticsForFirstDeclarator);
+                    if (fieldsBeingBound.ContainsReference(this))
+                    {
+                        diagnostics.Add(ErrorCode.ERR_CircularField, this.ErrorLocation, this);
+                        type = binder.CreateErrorType("circular");
+                    }
+                    else
+                    {
+                        type = binder.BindType(typeSyntax, diagnosticsForFirstDeclarator, fieldsBeingBound: fieldsBeingBound.Push(this));
+                    }
                 }
                 else
                 {

diff --git a/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs
@@ -673,12 +673,9 @@ internal override ImmutableArray<NamedTypeSymbol> InterfacesNoUseSiteDiagnostics
             return _underlyingType.InterfacesNoUseSiteDiagnostics(basesBeingResolved);
         }
 
-        internal sealed override bool IsManagedType
+        internal sealed override bool IsManagedType(ConsList<FieldSymbol> fieldsBeingBound)
         {
-            get
-            {
-                return _underlyingType.IsManagedType;
-            }
+            return _underlyingType.IsManagedType(fieldsBeingBound);
         }
 
         public override bool IsTupleType

diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/TypeParameterSymbol.cs
@@ -461,12 +461,9 @@ public sealed override bool IsValueType
             }
         }
 
-        internal sealed override bool IsManagedType
+        internal sealed override bool IsManagedType(ConsList<FieldSymbol> fieldsBeingBound)
         {
-            get
-            {
-                return !this.HasUnmanagedTypeConstraint;
-            }
+            return !this.HasUnmanagedTypeConstraint;
         }
 
         internal sealed override bool IsByRefLikeType

diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
@@ -614,7 +614,7 @@ public virtual NamedTypeSymbol TupleUnderlyingType
         /// <remarks>
         /// See Type::computeManagedType.
         /// </remarks>
-        internal abstract bool IsManagedType { get; }
+        internal abstract bool IsManagedType(ConsList<FieldSymbol> fieldsBeingBound = null);
 
         /// <summary>
         /// Returns true if the type may contain embedded references

diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="cs" original="../CSharpResources.resx">
     <body>
+      <trans-unit id="ERR_CircularField">
+        <source>The definition of field '{0}' is circular</source>
+        <target state="new">The definition of field '{0}' is circular</target>
+        <note />
+      </trans-unit>
       <trans-unit id="IDS_NULL">
         <source>&lt;null&gt;</source>
         <target state="translated">&lt;null&gt;</target>

diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="de" original="../CSharpResources.resx">
     <body>
+      <trans-unit id="ERR_CircularField">
+        <source>The definition of field '{0}' is circular</source>
+        <target state="new">The definition of field '{0}' is circular</target>
+        <note />
+      </trans-unit>
       <trans-unit id="IDS_NULL">
         <source>&lt;null&gt;</source>
         <target state="translated">&lt;NULL&gt;</target>

diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf
@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="es" original="../CSharpResources.resx">
     <body>
+      <trans-unit id="ERR_CircularField">
+        <source>The definition of field '{0}' is circular</source>
+        <target state="new">The definition of field '{0}' is circular</target>
+        <note />
+      </trans-unit>
       <trans-unit id="IDS_NULL">
         <source>&lt;null&gt;</source>
         <target state="translated">&lt;NULL&gt;</target>

diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf
@@ -2,6 +2,11 @@
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
   <file datatype="xml" source-language="en" target-language="fr" original="../CSharpResources.resx">
     <body>
+      <trans-unit id="ERR_CircularField">
+        <source>The definition of field '{0}' is circular</source>
+        <target state="new">The definition of field '{0}' is circular</target>
+        <note />
+      </trans-unit>
       <trans-unit id="IDS_NULL">
         <source>&lt;null&gt;</source>
         <target state="translated">&lt;Null&gt;</target>