Unify handling of directive parsing accross multiple code paths (#63832)

* Unify handling of directive parsing accross multiple code paths

* Feedback and fix

src/Compilers/CSharp/Portable/Parser/Directives.cs

@@ -3,7 +3,9 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 using System.Text;
+using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Text;
@@ -116,7 +118,6 @@ internal enum DefineState
     internal readonly struct DirectiveStack
     {
         public static readonly DirectiveStack Empty = new DirectiveStack(ConsList<Directive>.Empty);
-        public static readonly DirectiveStack Null = new DirectiveStack(null);
 
         private readonly ConsList<Directive>? _directives;
 
@@ -125,6 +126,9 @@ private DirectiveStack(ConsList<Directive>? directives)
             _directives = directives;
         }
 
+        public static void InterlockedInitialize(ref DirectiveStack location, DirectiveStack value)
+            => Interlocked.CompareExchange(ref Unsafe.AsRef(in location._directives), value._directives, null);
+
         public bool IsNull
         {
             get
@@ -351,8 +355,18 @@ private static ConsList<Directive> CompleteRegion(ConsList<Directive> stack)
             return current;
         }
 
-        private string GetDebuggerDisplay()
+        internal string GetDebuggerDisplay()
         {
+            if (IsNull)
+            {
+                return "<null>";
+            }
+
+            if (IsEmpty)
+            {
+                return "[]";
+            }
+
             var sb = new StringBuilder();
             for (var current = _directives; current != null && current.Any(); current = current.Tail)
             {

src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.LazySyntaxTree.cs

@@ -120,7 +120,7 @@ public override SyntaxTree WithRootAndOptions(SyntaxNode root, ParseOptions opti
                     _path,
                     (CSharpParseOptions)options,
                     (CSharpSyntaxNode)root,
-                    _directives,
+                    _lazyDirectives,
                     _diagnosticOptions,
                     cloneRoot: true);
             }
@@ -141,7 +141,7 @@ public override SyntaxTree WithFilePath(string path)
                         path,
                         _options,
                         root,
-                        GetDirectives(),
+                        directives: default,
                         _diagnosticOptions,
                         cloneRoot: true);
                 }
@@ -173,7 +173,7 @@ public override SyntaxTree WithDiagnosticOptions(ImmutableDictionary<string, Rep
                         _path,
                         _options,
                         root,
-                        GetDirectives(),
+                        directives: default,
                         options,
                         cloneRoot: true);
                 }

src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.ParsedSyntaxTree.cs

@@ -38,6 +38,7 @@ internal ParsedSyntaxTree(
                 Syntax.InternalSyntax.DirectiveStack directives,
                 ImmutableDictionary<string, ReportDiagnostic>? diagnosticOptions,
                 bool cloneRoot)
+                : base(directives)
             {
                 Debug.Assert(root != null);
                 Debug.Assert(options != null);
@@ -51,8 +52,6 @@ internal ParsedSyntaxTree(
                 _root = cloneRoot ? this.CloneNodeAsRoot(root) : root;
                 _hasCompilationUnitRoot = root.Kind() == SyntaxKind.CompilationUnit;
                 _diagnosticOptions = diagnosticOptions ?? EmptyDiagnosticOptions;
-
-                this.SetDirectiveStack(directives);
             }
 
             public override string FilePath
@@ -135,7 +134,7 @@ public override SyntaxTree WithRootAndOptions(SyntaxNode root, ParseOptions opti
                     _path,
                     (CSharpParseOptions)options,
                     (CSharpSyntaxNode)root,
-                    _directives,
+                    ReferenceEquals(_root, root) ? _lazyDirectives : default,
                     _diagnosticOptions,
                     cloneRoot: true);
             }
@@ -154,7 +153,7 @@ public override SyntaxTree WithFilePath(string path)
                     path,
                     _options,
                     _root,
-                    _directives,
+                    _lazyDirectives,
                     _diagnosticOptions,
                     cloneRoot: true);
             }
@@ -179,7 +178,7 @@ public override SyntaxTree WithDiagnosticOptions(ImmutableDictionary<string, Rep
                     _path,
                     _options,
                     _root,
-                    _directives,
+                    _lazyDirectives,
                     options,
                     cloneRoot: true);
             }

src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.cs

@@ -28,6 +28,29 @@ public abstract partial class CSharpSyntaxTree : SyntaxTree
     {
         internal static readonly SyntaxTree Dummy = new DummySyntaxTree();
 
+        private InternalSyntax.DirectiveStack _lazyDirectives;
+
+        /// <summary>
+        /// Stores positions where preprocessor state changes. Sorted by position.
+        /// The updated state can be found in <see cref="_preprocessorStates"/> array at the same index.
+        /// </summary>
+        private ImmutableArray<int> _preprocessorStateChangePositions;
+
+        /// <summary>
+        /// Preprocessor states corresponding to positions in <see cref="_preprocessorStateChangePositions"/>.
+        /// </summary>
+        private ImmutableArray<InternalSyntax.DirectiveStack> _preprocessorStates;
+
+        public CSharpSyntaxTree()
+            : this(directives: default)
+        {
+        }
+
+        internal CSharpSyntaxTree(InternalSyntax.DirectiveStack directives)
+        {
+            _lazyDirectives = directives;
+        }
+
         /// <summary>
         /// The options used by the parser to produce the syntax tree.
         /// </summary>
@@ -132,33 +155,26 @@ internal bool HasReferenceOrLoadDirectives
         }
 
         #region Preprocessor Symbols
-        private bool _hasDirectives;
-        private InternalSyntax.DirectiveStack _directives;
 
-        internal void SetDirectiveStack(InternalSyntax.DirectiveStack directives)
+        internal InternalSyntax.DirectiveStack GetDirectives()
         {
-            _directives = directives;
-            _hasDirectives = true;
-        }
-
-        private InternalSyntax.DirectiveStack GetDirectives()
-        {
-            if (!_hasDirectives)
+            if (_lazyDirectives.IsNull)
             {
-                var stack = this.GetRoot().CsGreen.ApplyDirectives(default);
-                SetDirectiveStack(stack);
+                InternalSyntax.DirectiveStack.InterlockedInitialize(ref _lazyDirectives, GetRoot().CsGreen.ApplyDirectives(InternalSyntax.DirectiveStack.Empty));
             }
 
-            return _directives;
+            Debug.Assert(!_lazyDirectives.IsNull);
+
+            return _lazyDirectives;
         }
 
         internal bool IsAnyPreprocessorSymbolDefined(ImmutableArray<string> conditionalSymbols)
         {
-            Debug.Assert(conditionalSymbols != null);
+            var directives = GetDirectives();
 
             foreach (string conditionalSymbol in conditionalSymbols)
             {
-                if (IsPreprocessorSymbolDefined(conditionalSymbol))
+                if (IsPreprocessorSymbolDefined(directives, conditionalSymbol))
                 {
                     return true;
                 }
@@ -167,11 +183,6 @@ internal bool IsAnyPreprocessorSymbolDefined(ImmutableArray<string> conditionalS
             return false;
         }
 
-        internal bool IsPreprocessorSymbolDefined(string symbolName)
-        {
-            return IsPreprocessorSymbolDefined(GetDirectives(), symbolName);
-        }
-
         private bool IsPreprocessorSymbolDefined(InternalSyntax.DirectiveStack directives, string symbolName)
         {
             switch (directives.IsDefined(symbolName))
@@ -185,17 +196,6 @@ private bool IsPreprocessorSymbolDefined(InternalSyntax.DirectiveStack directive
             }
         }
 
-        /// <summary>
-        /// Stores positions where preprocessor state changes. Sorted by position.
-        /// The updated state can be found in <see cref="_preprocessorStates"/> array at the same index.
-        /// </summary>
-        private ImmutableArray<int> _preprocessorStateChangePositions;
-
-        /// <summary>
-        /// Preprocessor states corresponding to positions in <see cref="_preprocessorStateChangePositions"/>.
-        /// </summary>
-        private ImmutableArray<InternalSyntax.DirectiveStack> _preprocessorStates;
-
         internal bool IsPreprocessorSymbolDefined(string symbolName, int position)
         {
             if (_preprocessorStateChangePositions.IsDefault)
@@ -342,18 +342,14 @@ public static SyntaxTree Create(
                 throw new ArgumentNullException(nameof(root));
             }
 
-            var directives = root.Kind() == SyntaxKind.CompilationUnit ?
-                ((CompilationUnitSyntax)root).GetConditionalDirectivesStack() :
-                InternalSyntax.DirectiveStack.Empty;
-
             return new ParsedSyntaxTree(
                 textOpt: null,
                 encodingOpt: encoding,
                 checksumAlgorithm: SourceHashAlgorithm.Sha1,
                 path: path,
                 options: options ?? CSharpParseOptions.Default,
                 root: root,
-                directives: directives,
+                directives: default,
                 diagnosticOptions,
                 cloneRoot: true);
         }
@@ -388,7 +384,7 @@ internal static SyntaxTree CreateWithoutClone(CSharpSyntaxNode root)
                 path: "",
                 options: CSharpParseOptions.Default,
                 root: root,
-                directives: InternalSyntax.DirectiveStack.Empty,
+                directives: default,
                 diagnosticOptions: null,
                 cloneRoot: false);
         }

src/Compilers/CSharp/Portable/Syntax/CompilationUnitSyntax.cs

@@ -69,30 +69,5 @@ private bool HasFirstTokenDirective(Func<SyntaxNode, bool> predicate)
 
             return false;
         }
-
-        internal Syntax.InternalSyntax.DirectiveStack GetConditionalDirectivesStack()
-        {
-            IEnumerable<DirectiveTriviaSyntax> directives = this.GetDirectives(filter: IsActiveConditionalDirective);
-            var directiveStack = Syntax.InternalSyntax.DirectiveStack.Empty;
-            foreach (DirectiveTriviaSyntax directive in directives)
-            {
-                var internalDirective = (Syntax.InternalSyntax.DirectiveTriviaSyntax)directive.Green;
-                directiveStack = internalDirective.ApplyDirectives(directiveStack);
-            }
-            return directiveStack;
-        }
-
-        private static bool IsActiveConditionalDirective(DirectiveTriviaSyntax directive)
-        {
-            switch (directive.Kind())
-            {
-                case SyntaxKind.DefineDirectiveTrivia:
-                    return ((DefineDirectiveTriviaSyntax)directive).IsActive;
-                case SyntaxKind.UndefDirectiveTrivia:
-                    return ((UndefDirectiveTriviaSyntax)directive).IsActive;
-                default:
-                    return false;
-            }
-        }
     }
 }

src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs

@@ -63,7 +63,7 @@ public void NoDuplicates()
         public void TestDiagnostic()
         {
             MockMessageProvider provider = new MockMessageProvider();
-            SyntaxTree syntaxTree = new MockSyntaxTree();
+            SyntaxTree syntaxTree = new MockCSharpSyntaxTree();
             CultureInfo englishCulture = CultureHelpers.EnglishCulture;
 
             DiagnosticInfo di1 = new DiagnosticInfo(provider, 1);
@@ -91,7 +91,7 @@ public void TestDiagnostic()
         public void TestCustomErrorInfo()
         {
             MockMessageProvider provider = new MockMessageProvider();
-            SyntaxTree syntaxTree = new MockSyntaxTree();
+            SyntaxTree syntaxTree = new MockCSharpSyntaxTree();
 
             DiagnosticInfo di3 = new CustomErrorInfo(provider, "OtherSymbol", new SourceLocation(syntaxTree, new TextSpan(14, 8)));
             var d3 = new CSDiagnostic(di3, new SourceLocation(syntaxTree, new TextSpan(1, 1)));