From ac0bc7015f75057e920a63e976162ab7360657d1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 14 Feb 2024 11:00:48 -0800 Subject: [PATCH 1/7] Add dedicated bit for indicating if a node itself has annotations (versus its children). --- .../Core/Portable/Syntax/GreenNode.cs | 48 ++++++++++++------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 54b3596801a9d..9d1bd6f9e4f93 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -97,7 +97,7 @@ protected GreenNode(ushort kind, DiagnosticInfo[]? diagnostics, SyntaxAnnotation if (annotation == null) throw new ArgumentException(paramName: nameof(annotations), message: "" /*CSharpResources.ElementsCannotBeNull*/); } - this.flags |= NodeFlags.ContainsAnnotations; + this.flags |= NodeFlags.HasAnnotations; s_annotationsTable.Add(this, annotations); } } @@ -112,7 +112,7 @@ protected GreenNode(ushort kind, DiagnosticInfo[]? diagnostics, SyntaxAnnotation if (annotation == null) throw new ArgumentException(paramName: nameof(annotations), message: "" /*CSharpResources.ElementsCannotBeNull*/); } - this.flags |= NodeFlags.ContainsAnnotations; + this.flags |= NodeFlags.HasAnnotations; s_annotationsTable.Add(this, annotations); } } @@ -262,12 +262,22 @@ internal enum NodeFlags : ushort ContainsStructuredTrivia = 1 << 1, ContainsDirectives = 1 << 2, ContainsSkippedText = 1 << 3, + /// + /// If this node, or any of its descendants has annotations attached to them. + /// ContainsAnnotations = 1 << 4, - IsNotMissing = 1 << 5, - ContainsAttributes = 1 << 6, - - FactoryContextIsInAsync = 1 << 7, - FactoryContextIsInQuery = 1 << 8, + /// + /// If this node itself has annotations (not just its descendants). + /// + HasAnnotations = (1 << 5) | ContainsAnnotations, + IsNotMissing = 1 << 6, + /// + /// If this node, or any of its descendants has attributes attached to it. + /// + ContainsAttributes = 1 << 7, + + FactoryContextIsInAsync = 1 << 8, + FactoryContextIsInQuery = 1 << 9, FactoryContextIsInIterator = FactoryContextIsInQuery, // VB does not use "InQuery", but uses "InIterator" instead InheritMask = ContainsDiagnostics | ContainsStructuredTrivia | ContainsDirectives | ContainsSkippedText | ContainsAnnotations | ContainsAttributes | IsNotMissing, @@ -368,6 +378,14 @@ public bool ContainsAnnotations return (this.flags & NodeFlags.ContainsAnnotations) != 0; } } + + public bool HasAnnotationsDirectly + { + get + { + return (this.flags & NodeFlags.HasAnnotations) != 0; + } + } #endregion #region Spans @@ -539,17 +557,13 @@ private static IEnumerable GetAnnotationsSlow(SyntaxAnnotation public SyntaxAnnotation[] GetAnnotations() { - if (this.ContainsAnnotations) - { - SyntaxAnnotation[]? annotations; - if (s_annotationsTable.TryGetValue(this, out annotations)) - { - System.Diagnostics.Debug.Assert(annotations.Length != 0, "we should return nonempty annotations or NoAnnotations"); - return annotations; - } - } + if (!this.HasAnnotationsDirectly) + return s_noAnnotations; - return s_noAnnotations; + var found = s_annotationsTable.TryGetValue(this, out var annotations); + Debug.Assert(found, "We must be able to find annotations since we had the bit set on ourselves"); + Debug.Assert(annotations.Length != 0, "we should return nonempty annotations or NoAnnotations"); + return annotations; } internal abstract GreenNode SetAnnotations(SyntaxAnnotation[]? annotations); From a23ad59770ff784f80dba949cf8173068f587e17 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 14 Feb 2024 12:17:38 -0800 Subject: [PATCH 2/7] nrt --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 9d1bd6f9e4f93..b29cb7b4a6999 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -562,7 +562,9 @@ public SyntaxAnnotation[] GetAnnotations() var found = s_annotationsTable.TryGetValue(this, out var annotations); Debug.Assert(found, "We must be able to find annotations since we had the bit set on ourselves"); - Debug.Assert(annotations.Length != 0, "we should return nonempty annotations or NoAnnotations"); + Debug.Assert(annotations != null, "annotations should not be null"); + Debug.Assert(annotations != s_noAnnotations, "annotations should not be s_noAnnotations"); + Debug.Assert(annotations.Length != 0, "annotations should be non-empty"); return annotations; } From cf2863cb536a60d554f846e096a19ed0248d73ad Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 14 Feb 2024 12:23:05 -0800 Subject: [PATCH 3/7] fix --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index b29cb7b4a6999..ccf0c2fa0ac97 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -383,7 +383,7 @@ public bool HasAnnotationsDirectly { get { - return (this.flags & NodeFlags.HasAnnotations) != 0; + return (this.flags & NodeFlags.HasAnnotations) == NodeFlags.HasAnnotations; } } #endregion From 47ba61553fe8dbd77fd219d4dbfdc820e363109e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 14 Feb 2024 12:26:51 -0800 Subject: [PATCH 4/7] Simplify --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index ccf0c2fa0ac97..bd7a44451eb3e 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -97,7 +97,7 @@ protected GreenNode(ushort kind, DiagnosticInfo[]? diagnostics, SyntaxAnnotation if (annotation == null) throw new ArgumentException(paramName: nameof(annotations), message: "" /*CSharpResources.ElementsCannotBeNull*/); } - this.flags |= NodeFlags.HasAnnotations; + this.flags |= (NodeFlags.HasAnnotations | NodeFlags.ContainsAnnotations); s_annotationsTable.Add(this, annotations); } } @@ -112,7 +112,7 @@ protected GreenNode(ushort kind, DiagnosticInfo[]? diagnostics, SyntaxAnnotation if (annotation == null) throw new ArgumentException(paramName: nameof(annotations), message: "" /*CSharpResources.ElementsCannotBeNull*/); } - this.flags |= NodeFlags.HasAnnotations; + this.flags |= (NodeFlags.HasAnnotations | NodeFlags.ContainsAnnotations); s_annotationsTable.Add(this, annotations); } } @@ -267,9 +267,9 @@ internal enum NodeFlags : ushort /// ContainsAnnotations = 1 << 4, /// - /// If this node itself has annotations (not just its descendants). + /// If this node directly has annotations (not its descendants). /// - HasAnnotations = (1 << 5) | ContainsAnnotations, + HasAnnotations = 1 << 5, IsNotMissing = 1 << 6, /// /// If this node, or any of its descendants has attributes attached to it. @@ -383,7 +383,7 @@ public bool HasAnnotationsDirectly { get { - return (this.flags & NodeFlags.HasAnnotations) == NodeFlags.HasAnnotations; + return (this.flags & NodeFlags.HasAnnotations) != 0; } } #endregion From 043a670608daf165936eab443ccc974a180fb8e5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 15 Feb 2024 10:42:24 -0800 Subject: [PATCH 5/7] whitespace --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 249597148c6fd..705213280fc65 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -388,6 +388,7 @@ public bool HasAnnotationsDirectly return (this.flags & NodeFlags.HasAnnotations) != 0; } } + #endregion #region Spans From 2fd71b7e2881414812e616ea2acba22fcbe33db7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 15 Feb 2024 10:43:07 -0800 Subject: [PATCH 6/7] REorder --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 705213280fc65..f888c20bbdab1 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -269,14 +269,14 @@ internal enum NodeFlags : ushort /// ContainsAnnotations = 1 << 4, /// - /// If this node directly has annotations (not its descendants). + /// If this node, or any of its descendants has attributes attached to it. /// - HasAnnotations = 1 << 5, - IsNotMissing = 1 << 6, + ContainsAttributes = 1 << 5, /// - /// If this node, or any of its descendants has attributes attached to it. + /// If this node directly has annotations (not its descendants). /// - ContainsAttributes = 1 << 7, + HasAnnotations = 1 << 6, + IsNotMissing = 1 << 7, FactoryContextIsInAsync = 1 << 8, FactoryContextIsInQuery = 1 << 9, From e88df21f4c59d638768f3e2b66d9683bdad140ef Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 15 Feb 2024 11:24:43 -0800 Subject: [PATCH 7/7] naming an ordering --- .../Core/Portable/Syntax/GreenNode.cs | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index f888c20bbdab1..13625bb2847fe 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -104,7 +104,7 @@ protected GreenNode(ushort kind, DiagnosticInfo[]? diagnostics, SyntaxAnnotation if (annotation == null) throw new ArgumentException(paramName: nameof(annotations), message: "" /*CSharpResources.ElementsCannotBeNull*/); } - this.flags |= (NodeFlags.HasAnnotations | NodeFlags.ContainsAnnotations); + this.flags |= (NodeFlags.HasAnnotationsDirectly | NodeFlags.ContainsAnnotations); s_annotationsTable.Add(this, annotations); } } @@ -119,7 +119,7 @@ protected GreenNode(ushort kind, DiagnosticInfo[]? diagnostics, SyntaxAnnotation if (annotation == null) throw new ArgumentException(paramName: nameof(annotations), message: "" /*CSharpResources.ElementsCannotBeNull*/); } - this.flags |= (NodeFlags.HasAnnotations | NodeFlags.ContainsAnnotations); + this.flags |= (NodeFlags.HasAnnotationsDirectly | NodeFlags.ContainsAnnotations); s_annotationsTable.Add(this, annotations); } } @@ -260,10 +260,25 @@ public virtual int FindSlotIndexContainingOffset(int offset) internal enum NodeFlags : ushort { None = 0, - ContainsDiagnostics = 1 << 0, - ContainsStructuredTrivia = 1 << 1, - ContainsDirectives = 1 << 2, - ContainsSkippedText = 1 << 3, + /// + /// If this node is missing or not. We use a non-zero value for the not-missing case so that this value + /// automatically merges upwards when building parent nodes. In other words, once we have one node that is + /// not-missing, all nodes above it are definitely not-missing as well. + /// + IsNotMissing = 1 << 0, + /// + /// If this node directly has annotations (not its descendants). can be + /// used to determine if a node or any of its descendants has annotations. + /// + HasAnnotationsDirectly = 1 << 1, + + FactoryContextIsInAsync = 1 << 2, + FactoryContextIsInQuery = 1 << 3, + FactoryContextIsInIterator = FactoryContextIsInQuery, // VB does not use "InQuery", but uses "InIterator" instead + + // Flags that are inherited upwards when building parent nodes. They should all start with "Contains" to + // indicate that the information could be found on it or anywhere in its children. + /// /// If this node, or any of its descendants has annotations attached to them. /// @@ -272,17 +287,12 @@ internal enum NodeFlags : ushort /// If this node, or any of its descendants has attributes attached to it. /// ContainsAttributes = 1 << 5, - /// - /// If this node directly has annotations (not its descendants). - /// - HasAnnotations = 1 << 6, - IsNotMissing = 1 << 7, - - FactoryContextIsInAsync = 1 << 8, - FactoryContextIsInQuery = 1 << 9, - FactoryContextIsInIterator = FactoryContextIsInQuery, // VB does not use "InQuery", but uses "InIterator" instead + ContainsDiagnostics = 1 << 6, + ContainsDirectives = 1 << 7, + ContainsSkippedText = 1 << 8, + ContainsStructuredTrivia = 1 << 9, - InheritMask = ContainsDiagnostics | ContainsStructuredTrivia | ContainsDirectives | ContainsSkippedText | ContainsAnnotations | ContainsAttributes | IsNotMissing, + InheritMask = IsNotMissing | ContainsAnnotations | ContainsAttributes | ContainsDiagnostics | ContainsDirectives | ContainsSkippedText | ContainsStructuredTrivia, } internal NodeFlags Flags @@ -385,7 +395,7 @@ public bool HasAnnotationsDirectly { get { - return (this.flags & NodeFlags.HasAnnotations) != 0; + return (this.flags & NodeFlags.HasAnnotationsDirectly) != 0; } }