Bypasses cycles by recognizing NonNullTypes syntactically

dotnet · Jun 16, 2018 · 3417d47 · 3417d47
1 parent ba396d5
commit 3417d47
Show file tree

Hide file tree

Showing 27 changed files with 160 additions and 289 deletions.
diff --git a/build/scripts/cibuild.cmd b/build/scripts/cibuild.cmd
@@ -1,5 +1,2 @@
 @echo off
-powershell -noprofile -executionPolicy RemoteSigned -file "%~dp0\build.ps1" -cibuild -build -restore -binaryLog %*
-
-REM PROTOTYPE(NullableReferenceTypes): Removed bootstrap because cycles crash CI
-REM powershell -noprofile -executionPolicy RemoteSigned -file "%~dp0\build.ps1" -cibuild -build -restore -bootstrap -binaryLog %*
+powershell -noprofile -executionPolicy RemoteSigned -file "%~dp0\build.ps1" -cibuild -build -restore -bootstrap -binaryLog %*
diff --git a/build/scripts/cibuild.sh b/build/scripts/cibuild.sh
@@ -24,7 +24,4 @@ export DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1
 echo "Building this commit:"
 git show --no-patch --pretty=raw HEAD
 
-"${root_path}"/build.sh --restore --build --packall --stop-vbcscompiler --test "$@"
-
-# PROTOTYPE(NullableReferenceTypes): Disabled bootstrap for now, because cycle issues block CI
-# "${root_path}"/build.sh --restore --bootstrap --build --packall --stop-vbcscompiler --test "$@"
+"${root_path}"/build.sh --restore --bootstrap --build --packall --stop-vbcscompiler --test "$@"
diff --git a/src/Compilers/CSharp/Portable/Binder/BinderFlags.cs b/src/Compilers/CSharp/Portable/Binder/BinderFlags.cs
@@ -8,7 +8,7 @@ namespace Microsoft.CodeAnalysis.CSharp
     /// A specific location for binding.
     /// </summary>
     [Flags]
-    internal enum BinderFlags : ulong
+    internal enum BinderFlags : uint
     {
         None, // No specific location
         SuppressConstraintChecks = 1 << 0,
@@ -113,7 +113,7 @@ internal enum BinderFlags : ulong
         /// Indicates a context with [NonNullTypes(false)], so unannotated reference types should be interpreted as null-oblivious.
         /// This flag is used to avoid cycles.
         /// </summary>
-        NonNullTypesFalse = (ulong)1 << 31,
+        NonNullTypesFalse = 1u << 31,
 
         // Groups
 

diff --git a/src/Compilers/CSharp/Portable/Binder/ContextualAttributeBinder.cs b/src/Compilers/CSharp/Portable/Binder/ContextualAttributeBinder.cs
@@ -15,9 +15,8 @@ internal sealed class ContextualAttributeBinder : Binder
 
         /// <param name="enclosing">Next binder in the chain (enclosing).</param>
         /// <param name="symbol">Symbol to which the attribute was applied (e.g. a parameter).</param>
-        /// Nullability does not matter in this context. Setting NonNullTypesFalse to avoid cycles.
         public ContextualAttributeBinder(Binder enclosing, Symbol symbol)
-            : base(enclosing, enclosing.Flags | BinderFlags.InContectualAttributeBinder | BinderFlags.NonNullTypesFalse)
+            : base(enclosing, enclosing.Flags | BinderFlags.InContectualAttributeBinder)
         {
             _attributeTarget = symbol;
             _attributedMember = GetAttributedMember(symbol);

diff --git a/.../CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/EventWellKnownAttributeData.cs b/.../CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/EventWellKnownAttributeData.cs
@@ -6,25 +6,5 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
 {
     internal sealed class EventWellKnownAttributeData : CommonEventWellKnownAttributeData
     {
-        #region NonNullTypesAttribute
-
-        private bool? _nonNullTypes;
-        public bool? NonNullTypes
-        {
-            get
-            {
-                VerifySealed(expected: true);
-                return _nonNullTypes;
-            }
-            set
-            {
-                VerifySealed(expected: false);
-                Debug.Assert(value.HasValue);
-                _nonNullTypes = value;
-                SetDataStored();
-            }
-        }
-
-        #endregion
     }
 }
diff --git a/.../CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/FieldWellKnownAttributeData.cs b/.../CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/FieldWellKnownAttributeData.cs
@@ -9,25 +9,5 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
     /// </summary>
     internal sealed class FieldWellKnownAttributeData : CommonFieldWellKnownAttributeData
     {
-        #region NonNullTypesAttribute
-
-        private bool? _nonNullTypes;
-        public bool? NonNullTypes
-        {
-            get
-            {
-                VerifySealed(expected: true);
-                return _nonNullTypes;
-            }
-            set
-            {
-                VerifySealed(expected: false);
-                Debug.Assert(value.HasValue);
-                _nonNullTypes = value;
-                SetDataStored();
-            }
-        }
-
-        #endregion
     }
 }
diff --git a/...p/Portable/Symbols/Attributes/WellKnownAttributeData/MethodEarlyWellKnownAttributeData.cs b/...p/Portable/Symbols/Attributes/WellKnownAttributeData/MethodEarlyWellKnownAttributeData.cs
@@ -12,25 +12,5 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
     /// </summary>
     internal sealed class MethodEarlyWellKnownAttributeData : CommonMethodEarlyWellKnownAttributeData
     {
-        #region NonNullTypesAttribute
-
-        private bool? _nonNullTypes;
-        public bool? NonNullTypes
-        {
-            get
-            {
-                VerifySealed(expected: true);
-                return _nonNullTypes;
-            }
-            set
-            {
-                VerifySealed(expected: false);
-                Debug.Assert(value.HasValue);
-                _nonNullTypes = value;
-                SetDataStored();
-            }
-        }
-
-        #endregion
     }
 }
diff --git a/...CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/ModuleWellKnownAttributeData.cs b/...CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/ModuleWellKnownAttributeData.cs
@@ -6,25 +6,5 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
 {
     internal sealed class ModuleWellKnownAttributeData : CommonModuleWellKnownAttributeData
     {
-        #region NonNullTypesAttribute
-
-        private bool? _nonNullTypes;
-        public bool? NonNullTypes
-        {
-            get
-            {
-                VerifySealed(expected: true);
-                return _nonNullTypes;
-            }
-            set
-            {
-                VerifySealed(expected: false);
-                Debug.Assert(value.HasValue);
-                _nonNullTypes = value;
-                SetDataStored();
-            }
-        }
-
-        #endregion
     }
 }
diff --git a/...Portable/Symbols/Attributes/WellKnownAttributeData/PropertyEarlyWellKnownAttributeData.cs b/...Portable/Symbols/Attributes/WellKnownAttributeData/PropertyEarlyWellKnownAttributeData.cs
@@ -39,26 +39,5 @@ public string IndexerName
         }
 
         #endregion
-
-        #region NonNullTypesAttribute
-
-        private bool? _nonNullTypes;
-        public bool? NonNullTypes
-        {
-            get
-            {
-                VerifySealed(expected: true);
-                return _nonNullTypes;
-            }
-            set
-            {
-                VerifySealed(expected: false);
-                Debug.Assert(value.HasValue);
-                _nonNullTypes = value;
-                SetDataStored();
-            }
-        }
-
-        #endregion
     }
 }
diff --git a/src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs
@@ -161,7 +161,6 @@ internal override bool NonNullTypes
         {
             get
             {
-                Debug.Assert(false); // PROTOTYPE(NullableReferenceTypes): Can we reach this?
                 Debug.Assert(IsDefinition);
 
                 var associatedSymbol = AssociatedSymbol;

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs b/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs
@@ -233,10 +233,6 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu
                 // NullableAttribute should not be set explicitly.
                 arguments.Diagnostics.Add(ErrorCode.ERR_ExplicitNullableAttribute, arguments.AttributeSyntaxOpt.Location);
             }
-            else if (attribute.IsTargetAttribute(this, AttributeDescription.NonNullTypesAttribute))
-            {
-                arguments.GetOrCreateData<FieldWellKnownAttributeData>().NonNullTypes = attribute.GetConstructorArgument<bool>(0, SpecialType.System_Boolean);
-            }
         }
 
         /// <summary>
@@ -344,8 +340,8 @@ internal sealed override bool NonNullTypes
         {
             get
             {
-                var data = GetDecodedWellKnownAttributeData();
-                return data?.NonNullTypes ?? ContainingType.NonNullTypes;
+                // PROTOTYPE(NullableReferenceTypes): temporary solution to avoid cycle
+                return SyntaxBasedNonNullTypes(this.AttributeDeclarationSyntaxList) ?? base.NonNullTypes; 
             }
         }
 

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbol.cs
@@ -79,15 +79,10 @@ protected override void MethodChecks(DiagnosticBag diagnostics)
             var binderFactory = this.DeclaringCompilation.GetBinderFactory(syntax.SyntaxTree);
             ParameterListSyntax parameterList = syntax.ParameterList;
 
-            // Force binding of early attributes to check for NonNullTypes attribute
-            BindNonNullTypesAttribute(syntax.AttributeLists);
-
-            var nonNullTypesFlag = NonNullTypes ? BinderFlags.NonNullTypesTrue : BinderFlags.NonNullTypesFalse;
-
             // NOTE: if we asked for the binder for the body of the constructor, we'd risk a stack overflow because
             // we might still be constructing the member list of the containing type.  However, getting the binder
             // for the parameters should be safe.
-            var bodyBinder = binderFactory.GetBinder(parameterList, syntax, this).WithAdditionalFlagsAndContainingMemberOrLambda(nonNullTypesFlag, this);
+            var bodyBinder = binderFactory.GetBinder(parameterList, syntax, this).WithContainingMemberOrLambda(this);
 
             SyntaxToken arglistToken;
             _lazyParameters = ParameterHelpers.MakeParameters(

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs
@@ -287,10 +287,6 @@ internal sealed override void DecodeWellKnownAttribute(ref DecodeWellKnownAttrib
             {
                 arguments.GetOrCreateData<EventWellKnownAttributeData>().HasSpecialNameAttribute = true;
             }
-            else if (attribute.IsTargetAttribute(this, AttributeDescription.NonNullTypesAttribute))
-            {
-                arguments.GetOrCreateData<EventWellKnownAttributeData>().NonNullTypes = attribute.GetConstructorArgument<bool>(0, SpecialType.System_Boolean);
-            }
             else if (attribute.IsTargetAttribute(this, AttributeDescription.NullableAttribute))
             {
                 // NullableAttribute should not be set explicitly.
@@ -346,8 +342,8 @@ internal override bool NonNullTypes
         {
             get
             {
-                var data = GetDecodedWellKnownAttributeData() as EventWellKnownAttributeData;
-                return data?.NonNullTypes ?? base.NonNullTypes;
+                // PROTOTYPE(NullableReferenceTypes): temporary solution to avoid cycle
+                return SyntaxBasedNonNullTypes(this.AttributeDeclarationSyntaxList) ?? base.NonNullTypes;
             }
         }
 

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs
@@ -163,7 +163,6 @@ public void EnsureMetadataVirtual()
 
         private CustomAttributesBag<CSharpAttributeData> _lazyCustomAttributesBag;
         private CustomAttributesBag<CSharpAttributeData> _lazyReturnTypeCustomAttributesBag;
-        private bool? _lazyNonNullTypesFromAttributes;
 
         private OverriddenOrHiddenMembersResult _lazyOverriddenOrHiddenMembers;
 
@@ -182,30 +181,12 @@ internal ImmutableArray<Diagnostic> Diagnostics
             get { return _cachedDiagnostics; }
         }
 
-        /// <summary>
-        /// Force binding of early attributes to check for NonNullTypes attribute
-        /// </summary>
-        protected void BindNonNullTypesAttribute(SyntaxList<AttributeListSyntax> attributes)
-        {
-            CustomAttributesBag<CSharpAttributeData> temp = null;
-            LoadAndValidateAttributes(OneOrMany.Create(attributes), ref temp, earlyDecodingOnly: true);
-            if (temp != null)
-            {
-                Debug.Assert(temp.IsEarlyDecodedWellKnownAttributeDataComputed);
-                var methodData = (MethodEarlyWellKnownAttributeData)temp.EarlyDecodedWellKnownAttributeData;
-                if (methodData != null)
-                {
-                    _lazyNonNullTypesFromAttributes = methodData.NonNullTypes;
-                }
-            }
-        }
-
         internal override bool NonNullTypes
         {
             get
             {
-                //return _lazyNonNullTypesFromAttributes ?? base.NonNullTypes; // PROTOTYPE(NullableReferenceTypes): breaking race condition for initializing parameters (see AttributeCtorInParam)
-                return _lazyNonNullTypesFromAttributes ?? ContainingModule?.UtilizesNullableReferenceTypes == true;
+                // PROTOTYPE(NullableReferenceTypes): temporary solution to avoid cycle
+                return SyntaxBasedNonNullTypes(this.GetAttributeDeclarations()) ?? base.NonNullTypes; 
             }
         }
 
@@ -1084,21 +1065,6 @@ internal override CSharpAttributeData EarlyDecodeWellKnownAttribute(ref EarlyDec
 
                     return null;
                 }
-                else if (CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.NonNullTypesAttribute))
-                {
-                    CSharpAttributeData boundAttribute = arguments.Binder.GetAttribute(arguments.AttributeSyntax, arguments.AttributeType, out hasAnyDiagnostics);
-                    if (!boundAttribute.HasErrors)
-                    {
-                        bool nonNullTypes = boundAttribute.CommonConstructorArguments[0].DecodeValue<bool>(SpecialType.System_Boolean);
-                        arguments.GetOrCreateData<MethodEarlyWellKnownAttributeData>().NonNullTypes = nonNullTypes;
-                        if (!hasAnyDiagnostics)
-                        {
-                            return boundAttribute;
-                        }
-                    }
-
-                    return null;
-                }
                 else
                 {
                     CSharpAttributeData boundAttribute;

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceModuleSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceModuleSymbol.cs
@@ -520,10 +520,6 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu
             {
                 DecodeOneNullableOptOutForAssemblyAttribute(arguments.AttributeSyntaxOpt, attribute, arguments.Diagnostics);
             }
-            else if (attribute.IsTargetAttribute(this, AttributeDescription.NonNullTypesAttribute))
-            {
-                arguments.GetOrCreateData<ModuleWellKnownAttributeData>().NonNullTypes = attribute.GetConstructorArgument<bool>(0, SpecialType.System_Boolean);
-            }
         }
 
         private void DecodeOneNullableOptOutForAssemblyAttribute(
@@ -591,8 +587,8 @@ internal override bool NonNullTypes
         {
             get
             {
-                var data = GetDecodedWellKnownAttributeData() as ModuleWellKnownAttributeData;
-                return data?.NonNullTypes ?? base.NonNullTypes;
+                // PROTOTYPE(NullableReferenceTypes): temporary solution to avoid cycle
+                return SyntaxBasedNonNullTypes(((SourceAssemblySymbol)this.ContainingAssembly).GetAttributeDeclarations()) ?? base.NonNullTypes;
             }
         }
 

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol.cs
@@ -520,22 +520,6 @@ internal override CSharpAttributeData EarlyDecodeWellKnownAttribute(ref EarlyDec
                 return null;
             }
 
-            if (CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.NonNullTypesAttribute))
-            {
-                boundAttribute = arguments.Binder.GetAttribute(arguments.AttributeSyntax, arguments.AttributeType, out hasAnyDiagnostics);
-                if (!boundAttribute.HasErrors)
-                {
-                    bool nonNullTypes = boundAttribute.CommonConstructorArguments[0].DecodeValue<bool>(SpecialType.System_Boolean);
-                    arguments.GetOrCreateData<CommonTypeEarlyWellKnownAttributeData>().NonNullTypes = nonNullTypes;
-                    if (!hasAnyDiagnostics)
-                    {
-                        return boundAttribute;
-                    }
-                }
-
-                return null;
-            }
-
             if (CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.CodeAnalysisEmbeddedAttribute))
             {
                 boundAttribute = arguments.Binder.GetAttribute(arguments.AttributeSyntax, arguments.AttributeType, out hasAnyDiagnostics);
@@ -929,8 +913,8 @@ internal override bool NonNullTypes
         {
             get
             {
-                var data = GetEarlyDecodedWellKnownAttributeData();
-                return  data?.NonNullTypes ?? base.NonNullTypes;
+                // PROTOTYPE(NullableReferenceTypes): temporary solution to avoid cycle
+                return SyntaxBasedNonNullTypes(this.GetAttributeDeclarations()) ?? base.NonNullTypes;
             }
         }
 

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs
@@ -350,13 +350,10 @@ private Tuple<NamedTypeSymbol, ImmutableArray<NamedTypeSymbol>> MakeOneDeclaredB
             var localInterfaces = ArrayBuilder<NamedTypeSymbol>.GetInstance();
             var baseBinder = this.DeclaringCompilation.GetBinder(bases);
 
-            // PROTOTYPE(NullableReferenceTypes): breaking cycle with hard-coded mitigation
-            var nonNullTypesFlag = (ContainingModule?.UtilizesNullableReferenceTypes == true) ? BinderFlags.NonNullTypesTrue : BinderFlags.NonNullTypesFalse;
-
             // Wrap base binder in a location-specific binder that will avoid generic constraint checks
             // (to avoid cycles if the constraint types are not bound yet). Instead, constraint checks
             // are handled by the caller.
-            baseBinder = baseBinder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks | nonNullTypesFlag, this);
+            baseBinder = baseBinder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks, this);
 
             int i = -1;
             foreach (var baseTypeSyntax in bases.Types)

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs
@@ -158,17 +158,12 @@ private void MethodChecks(MethodDeclarationSyntax syntax, Binder withTypeParamsB
 
             SyntaxToken arglistToken;
 
-            // Force binding of early attributes to check for NonNullTypes attribute
-            BindNonNullTypesAttribute(syntax.AttributeLists);
-
-            var nonNullTypesFlag = NonNullTypes ? BinderFlags.NonNullTypesTrue : BinderFlags.NonNullTypesFalse;
-
             // Constraint checking for parameter and return types must be delayed until
             // the method has been added to the containing type member list since
             // evaluating the constraints may depend on accessing this method from
             // the container (comparing this method to others to find overrides for
             // instance). Constraints are checked in AfterAddingTypeMembersChecks.
-            var signatureBinder = withTypeParamsBinder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks | nonNullTypesFlag, this);
+            var signatureBinder = withTypeParamsBinder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks, this);
 
             _lazyParameters = ParameterHelpers.MakeParameters(
                 signatureBinder, this, syntax.ParameterList, out arglistToken,