From c71fa5419aa035a8d76c8a82387353e3892180a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Matou=C5=A1ek?= Date: Sun, 11 Sep 2022 20:56:46 -0700 Subject: [PATCH] Unify handling of directive parsing accross multiple code paths (#63832) * Unify handling of directive parsing accross multiple code paths * Feedback and fix --- .../CSharp/Portable/Parser/Directives.cs | 18 +++- .../Syntax/CSharpSyntaxTree.LazySyntaxTree.cs | 6 +- .../CSharpSyntaxTree.ParsedSyntaxTree.cs | 9 +- .../Portable/Syntax/CSharpSyntaxTree.cs | 70 +++++++------- .../Portable/Syntax/CompilationUnitSyntax.cs | 25 ----- .../DiagnosticTest.MockSyntaxTree.cs | 90 ------------------ .../Test/Syntax/Diagnostics/DiagnosticTest.cs | 4 +- .../Syntax/Mocks/MockCSharpSyntaxTree.cs | 69 ++++++++++++++ .../Test/Syntax/Syntax/SyntaxTreeTests.cs | 93 ++++++++++++++++++- 9 files changed, 219 insertions(+), 165 deletions(-) delete mode 100644 src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.MockSyntaxTree.cs create mode 100644 src/Compilers/CSharp/Test/Syntax/Syntax/Mocks/MockCSharpSyntaxTree.cs diff --git a/src/Compilers/CSharp/Portable/Parser/Directives.cs b/src/Compilers/CSharp/Portable/Parser/Directives.cs index 66facf804d74b..30d0149a20605 100644 --- a/src/Compilers/CSharp/Portable/Parser/Directives.cs +++ b/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.Empty); - public static readonly DirectiveStack Null = new DirectiveStack(null); private readonly ConsList? _directives; @@ -125,6 +126,9 @@ private DirectiveStack(ConsList? 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 CompleteRegion(ConsList stack) return current; } - private string GetDebuggerDisplay() + internal string GetDebuggerDisplay() { + if (IsNull) + { + return ""; + } + + if (IsEmpty) + { + return "[]"; + } + var sb = new StringBuilder(); for (var current = _directives; current != null && current.Any(); current = current.Tail) { diff --git a/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.LazySyntaxTree.cs b/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.LazySyntaxTree.cs index cdbe590c9a043..c0588ba0aaa36 100644 --- a/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.LazySyntaxTree.cs +++ b/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? 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 + /// Stores positions where preprocessor state changes. Sorted by position. + /// The updated state can be found in array at the same index. + /// + private ImmutableArray _preprocessorStateChangePositions; + + /// + /// Preprocessor states corresponding to positions in . + /// + private ImmutableArray _preprocessorStates; + + public CSharpSyntaxTree() + : this(directives: default) + { + } + + internal CSharpSyntaxTree(InternalSyntax.DirectiveStack directives) + { + _lazyDirectives = directives; + } + /// /// The options used by the parser to produce the syntax tree. /// @@ -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 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 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 } } - /// - /// Stores positions where preprocessor state changes. Sorted by position. - /// The updated state can be found in array at the same index. - /// - private ImmutableArray _preprocessorStateChangePositions; - - /// - /// Preprocessor states corresponding to positions in . - /// - private ImmutableArray _preprocessorStates; - internal bool IsPreprocessorSymbolDefined(string symbolName, int position) { if (_preprocessorStateChangePositions.IsDefault) @@ -342,10 +342,6 @@ 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, @@ -353,7 +349,7 @@ public static SyntaxTree Create( 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); } diff --git a/src/Compilers/CSharp/Portable/Syntax/CompilationUnitSyntax.cs b/src/Compilers/CSharp/Portable/Syntax/CompilationUnitSyntax.cs index cb387fd8d7954..e3bc236f58330 100644 --- a/src/Compilers/CSharp/Portable/Syntax/CompilationUnitSyntax.cs +++ b/src/Compilers/CSharp/Portable/Syntax/CompilationUnitSyntax.cs @@ -69,30 +69,5 @@ private bool HasFirstTokenDirective(Func predicate) return false; } - - internal Syntax.InternalSyntax.DirectiveStack GetConditionalDirectivesStack() - { - IEnumerable 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; - } - } } } diff --git a/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.MockSyntaxTree.cs b/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.MockSyntaxTree.cs deleted file mode 100644 index 1a33cee8c15c6..0000000000000 --- a/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.MockSyntaxTree.cs +++ /dev/null @@ -1,90 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -#nullable disable - -using System; -using Microsoft.CodeAnalysis.CSharp.Symbols; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Text; -using System.Threading; -using System.Text; -using System.Collections.Immutable; - -namespace Microsoft.CodeAnalysis.CSharp.UnitTests -{ - public partial class DiagnosticTest - { - internal class MockSyntaxTree : CSharpSyntaxTree - { - public override string FilePath - { - get - { - return ""; - } - } - - public override SourceText GetText(CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public override bool TryGetText(out SourceText text) - { - throw new NotImplementedException(); - } - - public override Encoding Encoding - { - get - { - throw new NotImplementedException(); - } - } - - public override int Length - { - get { throw new NotImplementedException(); } - } - - public override CSharpParseOptions Options - { - get - { - throw new NotImplementedException(); - } - } - - public override CSharpSyntaxNode GetRoot(CancellationToken cancellationToken) - { - throw new NotImplementedException(); - } - - public override bool TryGetRoot(out CSharpSyntaxNode root) - { - throw new NotImplementedException(); - } - - public override SyntaxReference GetReference(SyntaxNode node) - { - throw new NotImplementedException(); - } - - public override SyntaxTree WithRootAndOptions(SyntaxNode root, ParseOptions options) - { - throw new NotImplementedException(); - } - - public override SyntaxTree WithFilePath(string path) - { - throw new NotImplementedException(); - } - public override bool HasCompilationUnitRoot - { - get { throw new NotImplementedException(); } - } - } - } -} diff --git a/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs b/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs index b0d5779f4e90c..9e1678ea76343 100644 --- a/src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs +++ b/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))); diff --git a/src/Compilers/CSharp/Test/Syntax/Syntax/Mocks/MockCSharpSyntaxTree.cs b/src/Compilers/CSharp/Test/Syntax/Syntax/Mocks/MockCSharpSyntaxTree.cs new file mode 100644 index 0000000000000..dca233877b49e --- /dev/null +++ b/src/Compilers/CSharp/Test/Syntax/Syntax/Mocks/MockCSharpSyntaxTree.cs @@ -0,0 +1,69 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Diagnostics.CodeAnalysis; +using System.Text; +using System.Threading; +using Microsoft.CodeAnalysis.CSharp.Test.Utilities; +using Microsoft.CodeAnalysis.Text; + +namespace Microsoft.CodeAnalysis.CSharp.UnitTests; + +internal class MockCSharpSyntaxTree : CSharpSyntaxTree +{ + private readonly SourceText _sourceText; + private readonly CSharpSyntaxNode? _root; + + public override CSharpParseOptions Options { get; } + public override string FilePath { get; } + + public MockCSharpSyntaxTree( + CSharpSyntaxNode? root = null, + SourceText? source = null, + CSharpParseOptions? options = null, + string? filePath = null) + { + _root = (root != null) ? CloneNodeAsRoot(root) : null; + _sourceText = source ?? SourceText.From("", Encoding.UTF8, SourceHashAlgorithm.Sha256); + Options = options ?? TestOptions.Regular; + FilePath = filePath ?? string.Empty; + } + + public override SourceText GetText(CancellationToken cancellationToken) + => _sourceText; + + public override bool TryGetText(out SourceText text) + { + text = _sourceText; + return true; + } + + public override Encoding? Encoding + => _sourceText.Encoding; + + public override int Length + => _sourceText.Length; + + public override CSharpSyntaxNode GetRoot(CancellationToken cancellationToken) + => _root ?? throw new NotImplementedException(); + + public override bool TryGetRoot([NotNullWhen(true)] out CSharpSyntaxNode? root) + { + root = _root; + return root != null; + } + + public override SyntaxTree WithRootAndOptions(SyntaxNode root, ParseOptions options) + => new MockCSharpSyntaxTree((CSharpSyntaxNode)root, _sourceText, (CSharpParseOptions)options, FilePath); + + public override SyntaxTree WithFilePath(string path) + => new MockCSharpSyntaxTree(_root, _sourceText, Options, FilePath); + + public override bool HasCompilationUnitRoot + => _root != null; + + public override SyntaxReference GetReference(SyntaxNode node) + => new SimpleSyntaxReference(node); +} diff --git a/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxTreeTests.cs b/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxTreeTests.cs index ab4c0f5cb8346..6db23aec9d70e 100644 --- a/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxTreeTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxTreeTests.cs @@ -5,11 +5,15 @@ #nullable disable using System.Collections.Immutable; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Text; +using System.Threading; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.Text; using Roslyn.Test.Utilities; +using Roslyn.Utilities; using Xunit; using Xunit.Abstractions; using static Roslyn.Test.Utilities.TestHelpers; @@ -21,10 +25,97 @@ public class SyntaxTreeTests : ParsingTests { public SyntaxTreeTests(ITestOutputHelper output) : base(output) { } + public enum SyntaxTreeFactoryKind + { + Create, + Subclass, + ParseText, + SynthesizedSyntaxTree, + ParsedTreeWithPath, + ParsedTreeWithRootAndOptions, + } + + [Theory] + [CombinatorialData] + public void SyntaxTreeCreationAndDirectiveParsing(SyntaxTreeFactoryKind factoryKind) + { + var source = """ + #define U + + #if !Q + #undef U + #endif + + #if U + #define X + #else + #define Y + #endif + + using System.Diagnostics; + + #if Y + class C + { + [Conditional("Y")] + public static void F1() + { + } + + [Conditional("U")] + public static void F2() + { + } + + public static void Main() + { + F1(); + F2(); + } + } + #endif + """; + + var parseOptions = CSharpParseOptions.Default; + var root = SyntaxFactory.ParseCompilationUnit(source, options: parseOptions); + + var tree = factoryKind switch + { + SyntaxTreeFactoryKind.Create => CSharpSyntaxTree.Create(root, options: parseOptions, path: "", encoding: null), + SyntaxTreeFactoryKind.ParseText => CSharpSyntaxTree.ParseText(SourceText.From(source, Encoding.UTF8, SourceHashAlgorithm.Sha256), parseOptions), + SyntaxTreeFactoryKind.Subclass => new MockCSharpSyntaxTree(root, SourceText.From(source, Encoding.UTF8, SourceHashAlgorithm.Sha256), parseOptions), + SyntaxTreeFactoryKind.SynthesizedSyntaxTree => SyntaxNode.CloneNodeAsRoot(root, syntaxTree: null).SyntaxTree, + SyntaxTreeFactoryKind.ParsedTreeWithPath => WithInitializedDirectives(CSharpSyntaxTree.Create(root, options: parseOptions, path: "old path", Encoding.UTF8)).WithFilePath("new path"), + SyntaxTreeFactoryKind.ParsedTreeWithRootAndOptions => WithInitializedDirectives(SyntaxFactory.ParseSyntaxTree("", options: parseOptions)).WithRootAndOptions(root, parseOptions), + _ => throw ExceptionUtilities.UnexpectedValue(factoryKind) + }; + + Assert.Equal("#define U | #undef U | #define Y", ((CSharpSyntaxTree)tree).GetDirectives().GetDebuggerDisplay()); + + var compilation = CSharpCompilation.Create("test", new[] { tree }, TargetFrameworkUtil.GetReferences(TargetFramework.Standard), TestOptions.DebugDll); + + CompileAndVerify(compilation).VerifyIL("C.Main", @" +{ + // Code size 8 (0x8) + .maxstack 0 + IL_0000: nop + IL_0001: call ""void C.F1()"" + IL_0006: nop + IL_0007: ret +} +"); + + static SyntaxTree WithInitializedDirectives(SyntaxTree tree) + { + _ = ((CSharpSyntaxTree)tree).GetDirectives(); + return tree; + } + } + // Diagnostic options on syntax trees are now obsolete #pragma warning disable CS0618 [Fact] - public void CreateTreeWithDiagnostics() + public void Create_WithDiagnosticOptions() { var options = CreateImmutableDictionary(("CS0078", ReportDiagnostic.Suppress)); var tree = CSharpSyntaxTree.Create(SyntaxFactory.ParseCompilationUnit(""),