From 6b6d0906944b1db4451086229b70f6dc0c495f04 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 12 Feb 2024 13:59:26 -0800 Subject: [PATCH 01/26] in progres --- .../Core/Portable/Syntax/GreenNode.cs | 82 +++++++++++++++++-- 1 file changed, 76 insertions(+), 6 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index ba2be0cb0b454..8c9790c344d6e 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -15,9 +15,69 @@ namespace Microsoft.CodeAnalysis { + [DebuggerDisplay("{GetDebuggerDisplay(), nq}")] internal abstract partial class GreenNode { + /// + /// Combination of and stored in a single 16bit value. + /// + internal struct NodeFlagsAndSlotCount + { + private const byte MaxEncodableSlotCount = 15; + + /// + /// 4 bits for the SlotCount. This allows slot counts of 1-14 to be stored as a direct byte. 15 indicates + /// that the slot count must be computed. + /// + private const ushort SlotCountMask = 0b0000000000001111; + + /// + /// 12 bits for the NodeFlags. This allows for up to 12 distinct bits to be stored to designate interesting + /// aspects of a node. + /// + private const ushort NodeFlagsMask = 0b1111111111110000; + + private const int NodeFlagsShift = 4; + + /// + /// First 12 bits are for NodeFlags. Last 4 are for SlotCount. + /// + private ushort _data; + + public byte SlotCount + { + readonly get + { + var result = (byte)(_data & SlotCountMask); + return result == MaxEncodableSlotCount ? byte.MaxValue : result; + } + + set + { + if (value == byte.MaxValue) + value = MaxEncodableSlotCount; + Debug.Assert(value <= 14); + + _data = (ushort)(_data | value); + } + } + + public NodeFlags NodeFlags + { + readonly get + { + return (NodeFlags)((_data & NodeFlagsMask) >> NodeFlagsShift); + } + + set + { + Debug.Assert((ushort)value <= (NodeFlagsMask >> NodeFlagsShift)); + _data = (ushort)(_data | ((ushort)value << NodeFlagsShift)); + } + } + } + private string GetDebuggerDisplay() { return this.GetType().Name + " " + this.KindText + " " + this.ToString(); @@ -26,10 +86,15 @@ private string GetDebuggerDisplay() internal const int ListKind = 1; private readonly ushort _kind; - protected NodeFlags flags; - private byte _slotCount; + protected NodeFlagsAndSlotCount _nodeFlagsAndSlotCount; private int _fullWidth; + protected NodeFlags flags + { + get => _nodeFlagsAndSlotCount.NodeFlags; + set => _nodeFlagsAndSlotCount.NodeFlags |= value; + } + private static readonly ConditionalWeakTable s_diagnosticsTable = new ConditionalWeakTable(); @@ -141,7 +206,7 @@ public int SlotCount { get { - int count = _slotCount; + int count = _nodeFlagsAndSlotCount.SlotCount; if (count == byte.MaxValue) { count = GetSlotCount(); @@ -152,7 +217,7 @@ public int SlotCount protected set { - _slotCount = (byte)value; + _nodeFlagsAndSlotCount.SlotCount = (byte)value; } } @@ -168,7 +233,7 @@ internal GreenNode GetRequiredSlot(int index) // for slot counts >= byte.MaxValue protected virtual int GetSlotCount() { - return _slotCount; + return _nodeFlagsAndSlotCount.SlotCount; } public virtual int GetSlotOffset(int index) @@ -234,8 +299,13 @@ public virtual int FindSlotIndexContainingOffset(int offset) #endregion #region Flags + + /// + /// Special flags a node can have. Note: while this is typed as being `ushort`, we can only practically use 12 + /// of those 16 bits as we use the remaining 4 bits to store the slot count of a node. + /// [Flags] - internal enum NodeFlags : byte + internal enum NodeFlags : ushort { None = 0, ContainsDiagnostics = 1 << 0, From b2e464454a13cf35c2a28c389db00191b0ae161a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 12 Feb 2024 14:05:01 -0800 Subject: [PATCH 02/26] Flip --- .../Core/Portable/Syntax/GreenNode.cs | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 8c9790c344d6e..c43becd545aba 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -24,24 +24,21 @@ internal abstract partial class GreenNode /// internal struct NodeFlagsAndSlotCount { - private const byte MaxEncodableSlotCount = 15; - /// - /// 4 bits for the SlotCount. This allows slot counts of 1-14 to be stored as a direct byte. 15 indicates - /// that the slot count must be computed. + /// 4 bits for the SlotCount. This allows slot counts of 0-14 to be stored as a direct byte. All 1s + /// indicates that the slot count must be computed. /// - private const ushort SlotCountMask = 0b0000000000001111; + private const byte SlotCountMask = 0b1111; + private const int SlotCountShift = 12; /// /// 12 bits for the NodeFlags. This allows for up to 12 distinct bits to be stored to designate interesting /// aspects of a node. /// - private const ushort NodeFlagsMask = 0b1111111111110000; - - private const int NodeFlagsShift = 4; + private const ushort NodeFlagsMask = 0b0000111111111111; /// - /// First 12 bits are for NodeFlags. Last 4 are for SlotCount. + /// CCCCFFFFFFFFFFFF for Count bits then Flag bits. /// private ushort _data; @@ -49,17 +46,19 @@ public byte SlotCount { readonly get { - var result = (byte)(_data & SlotCountMask); - return result == MaxEncodableSlotCount ? byte.MaxValue : result; + var shifted = _data >> SlotCountShift; + Debug.Assert(shifted <= SlotCountMask); + var result = (byte)shifted; + return result == SlotCountMask ? byte.MaxValue : result; } set { if (value == byte.MaxValue) - value = MaxEncodableSlotCount; - Debug.Assert(value <= 14); + value = SlotCountMask; + Debug.Assert(value <= SlotCountMask); - _data = (ushort)(_data | value); + _data = (ushort)(_data | (value << SlotCountShift)); } } @@ -67,13 +66,13 @@ public NodeFlags NodeFlags { readonly get { - return (NodeFlags)((_data & NodeFlagsMask) >> NodeFlagsShift); + return (NodeFlags)(_data & NodeFlagsMask); } set { - Debug.Assert((ushort)value <= (NodeFlagsMask >> NodeFlagsShift)); - _data = (ushort)(_data | ((ushort)value << NodeFlagsShift)); + Debug.Assert((ushort)value <= NodeFlagsMask); + _data = (ushort)(_data | (ushort)value); } } } From a7b0f5c7f7d2f6fa71d829a769f30bfb1692045d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 12 Feb 2024 14:08:20 -0800 Subject: [PATCH 03/26] Drop in replacement --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index c43becd545aba..5af9adc1f6b4c 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -85,7 +85,7 @@ private string GetDebuggerDisplay() internal const int ListKind = 1; private readonly ushort _kind; - protected NodeFlagsAndSlotCount _nodeFlagsAndSlotCount; + private NodeFlagsAndSlotCount _nodeFlagsAndSlotCount; private int _fullWidth; protected NodeFlags flags @@ -94,6 +94,12 @@ protected NodeFlags flags set => _nodeFlagsAndSlotCount.NodeFlags |= value; } + private byte _slotCount + { + get => _nodeFlagsAndSlotCount.SlotCount; + set => _nodeFlagsAndSlotCount.SlotCount = value; + } + private static readonly ConditionalWeakTable s_diagnosticsTable = new ConditionalWeakTable(); @@ -205,7 +211,7 @@ public int SlotCount { get { - int count = _nodeFlagsAndSlotCount.SlotCount; + int count = _slotCount; if (count == byte.MaxValue) { count = GetSlotCount(); @@ -216,7 +222,7 @@ public int SlotCount protected set { - _nodeFlagsAndSlotCount.SlotCount = (byte)value; + _slotCount = (byte)value; } } @@ -232,7 +238,7 @@ internal GreenNode GetRequiredSlot(int index) // for slot counts >= byte.MaxValue protected virtual int GetSlotCount() { - return _nodeFlagsAndSlotCount.SlotCount; + return _slotCount; } public virtual int GetSlotOffset(int index) From b90d0fbda6c164fbddc1960a4e8626e19ca96e5c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 12 Feb 2024 15:23:49 -0800 Subject: [PATCH 04/26] Update src/Compilers/Core/Portable/Syntax/GreenNode.cs --- 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 5af9adc1f6b4c..789d58cdbfe40 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -91,7 +91,7 @@ private string GetDebuggerDisplay() protected NodeFlags flags { get => _nodeFlagsAndSlotCount.NodeFlags; - set => _nodeFlagsAndSlotCount.NodeFlags |= value; + set => _nodeFlagsAndSlotCount.NodeFlags = value; } private byte _slotCount From ebd279d445cf13d5cd2a5eca0c8cdff91b6250c4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 09:41:17 -0800 Subject: [PATCH 05/26] fix --- .../Core/Portable/Syntax/GreenNode.cs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 789d58cdbfe40..2b29bf076304a 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -28,14 +28,16 @@ internal struct NodeFlagsAndSlotCount /// 4 bits for the SlotCount. This allows slot counts of 0-14 to be stored as a direct byte. All 1s /// indicates that the slot count must be computed. /// - private const byte SlotCountMask = 0b1111; + private const ushort SlotCountMask = 0b1111000000000000; + private const ushort NodeFlagsMask = 0b0000111111111111; + private const int SlotCountShift = 12; + private const int MaxSlotCount = 15; /// /// 12 bits for the NodeFlags. This allows for up to 12 distinct bits to be stored to designate interesting /// aspects of a node. /// - private const ushort NodeFlagsMask = 0b0000111111111111; /// /// CCCCFFFFFFFFFFFF for Count bits then Flag bits. @@ -47,18 +49,19 @@ public byte SlotCount readonly get { var shifted = _data >> SlotCountShift; - Debug.Assert(shifted <= SlotCountMask); + Debug.Assert(shifted <= MaxSlotCount); var result = (byte)shifted; - return result == SlotCountMask ? byte.MaxValue : result; + return result == MaxSlotCount ? byte.MaxValue : result; } set { if (value == byte.MaxValue) - value = SlotCountMask; - Debug.Assert(value <= SlotCountMask); + value = MaxSlotCount; + Debug.Assert(value <= MaxSlotCount); - _data = (ushort)(_data | (value << SlotCountShift)); + // Clear out everything but the node-flags, and then assign into the slot-count segment. + _data = (ushort)((_data & NodeFlagsMask) | (value << SlotCountShift)); } } @@ -72,7 +75,9 @@ readonly get set { Debug.Assert((ushort)value <= NodeFlagsMask); - _data = (ushort)(_data | (ushort)value); + + // Clear out everything but the slot-count, and then assign into the node-flags segment. + _data = (ushort)((_data & SlotCountMask) | (ushort)value); } } } From 9e91de514a9c6100d6a17a002928e94d4d5612f6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 09:57:08 -0800 Subject: [PATCH 06/26] Add specific list test --- .../CollectionExpressionParsingTests.cs | 32 +++++++++++++++++++ .../Core/Portable/Syntax/GreenNode.cs | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/CollectionExpressionParsingTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/CollectionExpressionParsingTests.cs index 53ac300301e9b..75cde7ce70fbc 100644 --- a/src/Compilers/CSharp/Test/Syntax/Parsing/CollectionExpressionParsingTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/CollectionExpressionParsingTests.cs @@ -2,6 +2,7 @@ // 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.Linq; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Roslyn.Test.Utilities; using Xunit; @@ -17096,4 +17097,35 @@ public void TreatKeywordAsCollectionExprElement() } EOF(); } + + [Theory, CombinatorialData] + public void CollectionExpressionParsingSlotCounts([CombinatorialRange(1, 20)] int count) + { + // Validate no errors for collections with small and large number of elements. Importantly, we want to test the + // boundary points where the slot count of the collection crosses over the amount that can be directly stored in + // the node, versus the slot count stored in subclass nodes. + var text = $"[{string.Join(", ", Enumerable.Range(1, count).Select(i => $"A{i}"))}]"; + + UsingExpression(text, TestOptions.Regular); + + N(SyntaxKind.CollectionExpression); + N(SyntaxKind.OpenBracketToken); + + for (var i = 1; i <= count; i++) + { + N(SyntaxKind.ExpressionElement); + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, $"A{i}"); + } + + if (i < count) + { + N(SyntaxKind.CommaToken); + } + } + + N(SyntaxKind.CloseBracketToken); + EOF(); + } } diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 2b29bf076304a..444f91caa7ec9 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -56,7 +56,7 @@ readonly get set { - if (value == byte.MaxValue) + if (value >= MaxSlotCount) value = MaxSlotCount; Debug.Assert(value <= MaxSlotCount); From f8c0abae1db43f69d3d0f4d0faa86d7f45b223b7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:00:39 -0800 Subject: [PATCH 07/26] Add attribute flag --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 444f91caa7ec9..384f02a8f936e 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -324,12 +324,13 @@ internal enum NodeFlags : ushort ContainsSkippedText = 1 << 3, ContainsAnnotations = 1 << 4, IsNotMissing = 1 << 5, + ContainsAttributes = 1 << 6, - FactoryContextIsInAsync = 1 << 6, - FactoryContextIsInQuery = 1 << 7, + FactoryContextIsInAsync = 1 << 7, + FactoryContextIsInQuery = 1 << 8, FactoryContextIsInIterator = FactoryContextIsInQuery, // VB does not use "InQuery", but uses "InIterator" instead - InheritMask = ContainsDiagnostics | ContainsStructuredTrivia | ContainsDirectives | ContainsSkippedText | ContainsAnnotations | IsNotMissing, + InheritMask = ContainsDiagnostics | ContainsStructuredTrivia | ContainsDirectives | ContainsSkippedText | ContainsAnnotations | ContainsAttributes | IsNotMissing, } internal NodeFlags Flags From 8e54ffb63049d40bf50cfb27741d9ef62595453e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:22:21 -0800 Subject: [PATCH 08/26] generators --- .../Source/CSharpSyntaxGenerator/SourceWriter.cs | 12 ++++++++++++ .../GreenNodes/GreenNodeWriter.vb | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs b/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs index 7f05d6743aa7d..9a4693a61901d 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs +++ b/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs @@ -173,6 +173,10 @@ private void WriteGreenType(TreeType node) WriteLine(", DiagnosticInfo[]? diagnostics, SyntaxAnnotation[]? annotations)"); WriteLine(" : base(kind, diagnostics, annotations)"); OpenBlock(); + if (node.Name == "AttributeSyntax") + { + WriteLine("this.Flags |= NodeFlags.ContainsAttributes;"); + } WriteCtorBody(valueFields, nodeFields); CloseBlock(); @@ -185,6 +189,10 @@ private void WriteGreenType(TreeType node) WriteLine(", SyntaxFactoryContext context)"); WriteLine(" : base(kind)"); OpenBlock(); + if (node.Name == "AttributeSyntax") + { + WriteLine("this.Flags |= NodeFlags.ContainsAttributes;"); + } WriteLine("this.SetFactoryContext(context);"); WriteCtorBody(valueFields, nodeFields); CloseBlock(); @@ -198,6 +206,10 @@ private void WriteGreenType(TreeType node) WriteLine(")"); WriteLine(" : base(kind)"); OpenBlock(); + if (node.Name == "AttributeSyntax") + { + WriteLine("this.Flags |= NodeFlags.ContainsAttributes;"); + } WriteCtorBody(valueFields, nodeFields); CloseBlock(); WriteLine(); diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb index f9030e0b901f2..450593fa8d523 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb +++ b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb @@ -377,6 +377,11 @@ Friend Class GreenNodeWriter End If End If + + If nodeStructure.Name = "AttributeSyntax" Then + _writer.WriteLine("Me.Flags = Me.Flags Or NodeFlags.ContainsAttributes") + End If + ' Generate code to initialize this class If contextual Then From f6087f93919cab02da487c1155adb0cc720667b3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:23:31 -0800 Subject: [PATCH 09/26] Better --- .../Syntax.xml.Internal.Generated.cs | 3 +++ .../GreenNodes/GreenNodeWriter.vb | 9 ++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs b/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs index 4631a0eea939f..eb7cfd4210a43 100644 --- a/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs +++ b/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs @@ -15864,6 +15864,7 @@ internal sealed partial class AttributeSyntax : CSharpSyntaxNode internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentListSyntax? argumentList, DiagnosticInfo[]? diagnostics, SyntaxAnnotation[]? annotations) : base(kind, diagnostics, annotations) { + this.Flags |= NodeFlags.ContainsAttributes; this.SlotCount = 2; this.AdjustFlagsAndWidth(name); this.name = name; @@ -15877,6 +15878,7 @@ internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentList internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentListSyntax? argumentList, SyntaxFactoryContext context) : base(kind) { + this.Flags |= NodeFlags.ContainsAttributes; this.SetFactoryContext(context); this.SlotCount = 2; this.AdjustFlagsAndWidth(name); @@ -15891,6 +15893,7 @@ internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentList internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentListSyntax? argumentList) : base(kind) { + this.Flags |= NodeFlags.ContainsAttributes; this.SlotCount = 2; this.AdjustFlagsAndWidth(name); this.name = name; diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb index 450593fa8d523..8f5483e9a15cf 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb +++ b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb @@ -377,11 +377,6 @@ Friend Class GreenNodeWriter End If End If - - If nodeStructure.Name = "AttributeSyntax" Then - _writer.WriteLine("Me.Flags = Me.Flags Or NodeFlags.ContainsAttributes") - End If - ' Generate code to initialize this class If contextual Then @@ -425,6 +420,10 @@ Friend Class GreenNodeWriter _writer.WriteLine(" SetFlags(NodeFlags.ContainsDirectives)") End If + If StructureTypeName(nodeStructure) = "AttributeSyntax" Then + _writer.WriteLine("Me.Flags = Me.Flags Or NodeFlags.ContainsAttributes") + End If + ' Generate End Sub _writer.WriteLine(" End Sub") _writer.WriteLine() From 2e10fc873c08d6628b3f67269c5b2da0ab04ac86 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:24:11 -0800 Subject: [PATCH 10/26] Better --- .../VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb index 8f5483e9a15cf..ee8f0b071d620 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb +++ b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb @@ -421,7 +421,7 @@ Friend Class GreenNodeWriter End If If StructureTypeName(nodeStructure) = "AttributeSyntax" Then - _writer.WriteLine("Me.Flags = Me.Flags Or NodeFlags.ContainsAttributes") + _writer.WriteLine(" SetFlags(NodeFlags.ContainsAttributes)") End If ' Generate End Sub From 37ffbe8d892f68db08c2b9d8992bf926925c11f6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:26:22 -0800 Subject: [PATCH 11/26] Better --- .../CSharpSyntaxGenerator/SourceWriter.cs | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs b/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs index 9a4693a61901d..b447015006eab 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs +++ b/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs @@ -173,11 +173,7 @@ private void WriteGreenType(TreeType node) WriteLine(", DiagnosticInfo[]? diagnostics, SyntaxAnnotation[]? annotations)"); WriteLine(" : base(kind, diagnostics, annotations)"); OpenBlock(); - if (node.Name == "AttributeSyntax") - { - WriteLine("this.Flags |= NodeFlags.ContainsAttributes;"); - } - WriteCtorBody(valueFields, nodeFields); + WriteCtorBody(nd, valueFields, nodeFields); CloseBlock(); // write constructor with async @@ -189,12 +185,8 @@ private void WriteGreenType(TreeType node) WriteLine(", SyntaxFactoryContext context)"); WriteLine(" : base(kind)"); OpenBlock(); - if (node.Name == "AttributeSyntax") - { - WriteLine("this.Flags |= NodeFlags.ContainsAttributes;"); - } WriteLine("this.SetFactoryContext(context);"); - WriteCtorBody(valueFields, nodeFields); + WriteCtorBody(nd, valueFields, nodeFields); CloseBlock(); // write constructor without diagnostics and annotations @@ -206,11 +198,7 @@ private void WriteGreenType(TreeType node) WriteLine(")"); WriteLine(" : base(kind)"); OpenBlock(); - if (node.Name == "AttributeSyntax") - { - WriteLine("this.Flags |= NodeFlags.ContainsAttributes;"); - } - WriteCtorBody(valueFields, nodeFields); + WriteCtorBody(nd, valueFields, nodeFields); CloseBlock(); WriteLine(); @@ -301,8 +289,13 @@ private void WriteGreenNodeConstructorArgs(List nodeFields, List v } } - private void WriteCtorBody(List valueFields, List nodeFields) + private void WriteCtorBody(Node node, List valueFields, List nodeFields) { + if (node.Name == "AttributeSyntax") + { + WriteLine("this.flags |= NodeFlags.ContainsAttributes;"); + } + // constructor body WriteLine($"this.SlotCount = {nodeFields.Count};"); From 4fd426bd2dfb147f00a054be543c21ed9e191878 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:48:29 -0800 Subject: [PATCH 12/26] fix --- .../Syntax.xml.Internal.Generated.cs | 30 ++++--------------- .../SourceGeneration/CSharpSyntaxHelper.cs | 5 ---- .../SourceGeneration/ISyntaxHelper.cs | 15 ---------- ...alueProvider_ForAttributeWithSimpleName.cs | 4 +++ .../Core/Portable/Syntax/SyntaxTree.cs | 2 +- .../VisualBasicSyntaxHelper.vb | 6 ---- .../CSharpSyntaxGenerator/SourceWriter.cs | 1 + .../GreenNodes/GreenNodeWriter.vb | 7 ++--- 8 files changed, 14 insertions(+), 56 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs b/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs index eb7cfd4210a43..640015f6944e8 100644 --- a/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs +++ b/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs @@ -15864,7 +15864,7 @@ internal sealed partial class AttributeSyntax : CSharpSyntaxNode internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentListSyntax? argumentList, DiagnosticInfo[]? diagnostics, SyntaxAnnotation[]? annotations) : base(kind, diagnostics, annotations) { - this.Flags |= NodeFlags.ContainsAttributes; + this.flags |= NodeFlags.ContainsAttributes; this.SlotCount = 2; this.AdjustFlagsAndWidth(name); this.name = name; @@ -15878,8 +15878,8 @@ internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentList internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentListSyntax? argumentList, SyntaxFactoryContext context) : base(kind) { - this.Flags |= NodeFlags.ContainsAttributes; this.SetFactoryContext(context); + this.flags |= NodeFlags.ContainsAttributes; this.SlotCount = 2; this.AdjustFlagsAndWidth(name); this.name = name; @@ -15893,7 +15893,7 @@ internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentList internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentListSyntax? argumentList) : base(kind) { - this.Flags |= NodeFlags.ContainsAttributes; + this.flags |= NodeFlags.ContainsAttributes; this.SlotCount = 2; this.AdjustFlagsAndWidth(name); this.name = name; @@ -30783,17 +30783,7 @@ public AttributeSyntax Attribute(NameSyntax name, AttributeArgumentListSyntax? a if (name == null) throw new ArgumentNullException(nameof(name)); #endif - int hash; - var cached = CSharpSyntaxNodeCache.TryGetNode((int)SyntaxKind.Attribute, name, argumentList, this.context, out hash); - if (cached != null) return (AttributeSyntax)cached; - - var result = new AttributeSyntax(SyntaxKind.Attribute, name, argumentList, this.context); - if (hash >= 0) - { - SyntaxNodeCache.AddNode(result, hash); - } - - return result; + return new AttributeSyntax(SyntaxKind.Attribute, name, argumentList, this.context); } public AttributeArgumentListSyntax AttributeArgumentList(SyntaxToken openParenToken, CoreSyntax.SeparatedSyntaxList arguments, SyntaxToken closeParenToken) @@ -35999,17 +35989,7 @@ public static AttributeSyntax Attribute(NameSyntax name, AttributeArgumentListSy if (name == null) throw new ArgumentNullException(nameof(name)); #endif - int hash; - var cached = SyntaxNodeCache.TryGetNode((int)SyntaxKind.Attribute, name, argumentList, out hash); - if (cached != null) return (AttributeSyntax)cached; - - var result = new AttributeSyntax(SyntaxKind.Attribute, name, argumentList); - if (hash >= 0) - { - SyntaxNodeCache.AddNode(result, hash); - } - - return result; + return new AttributeSyntax(SyntaxKind.Attribute, name, argumentList); } public static AttributeArgumentListSyntax AttributeArgumentList(SyntaxToken openParenToken, CoreSyntax.SeparatedSyntaxList arguments, SyntaxToken closeParenToken) diff --git a/src/Compilers/CSharp/Portable/SourceGeneration/CSharpSyntaxHelper.cs b/src/Compilers/CSharp/Portable/SourceGeneration/CSharpSyntaxHelper.cs index 4014712c69783..3be1c2ba29827 100644 --- a/src/Compilers/CSharp/Portable/SourceGeneration/CSharpSyntaxHelper.cs +++ b/src/Compilers/CSharp/Portable/SourceGeneration/CSharpSyntaxHelper.cs @@ -2,10 +2,8 @@ // 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.Threading; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.PooledObjects; -using Microsoft.CodeAnalysis.SourceGeneration; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp @@ -21,9 +19,6 @@ private CSharpSyntaxHelper() public override bool IsCaseSensitive => true; - protected override int AttributeListKind - => (int)SyntaxKind.AttributeList; - public override bool IsValidIdentifier(string name) => SyntaxFacts.IsValidIdentifier(name); diff --git a/src/Compilers/Core/Portable/SourceGeneration/ISyntaxHelper.cs b/src/Compilers/Core/Portable/SourceGeneration/ISyntaxHelper.cs index 52906bd178683..9e9a46124b152 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/ISyntaxHelper.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/ISyntaxHelper.cs @@ -32,14 +32,12 @@ internal interface ISyntaxHelper void AddAliases(GreenNode node, ArrayBuilder<(string aliasName, string symbolName)> aliases, bool global); void AddAliases(CompilationOptions options, ArrayBuilder<(string aliasName, string symbolName)> aliases); - bool ContainsAttributeList(SyntaxNode root); bool ContainsGlobalAliases(SyntaxNode root); } internal abstract class AbstractSyntaxHelper : ISyntaxHelper { public abstract bool IsCaseSensitive { get; } - protected abstract int AttributeListKind { get; } public abstract bool IsValidIdentifier(string name); @@ -60,18 +58,5 @@ internal abstract class AbstractSyntaxHelper : ISyntaxHelper public abstract void AddAliases(CompilationOptions options, ArrayBuilder<(string aliasName, string symbolName)> aliases); public abstract bool ContainsGlobalAliases(SyntaxNode root); - - public bool ContainsAttributeList(SyntaxNode root) - { - var attributeListKind = this.AttributeListKind; - - foreach (var node in root.Green.EnumerateNodes()) - { - if (node.RawKind == attributeListKind) - return true; - } - - return false; - } } } diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs index c3b7b48cba0c1..ae6152f2dd267 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs @@ -234,6 +234,10 @@ void processMember(SyntaxNode member) { cancellationToken.ThrowIfCancellationRequested(); + // Don't bother descending into nodes that don't contain attributes. + if (!member.Green.Flags.HasFlag(GreenNode.NodeFlags.ContainsAttributes)) + return; + // nodes can be arbitrarily deep. Use an explicit stack over recursion to prevent a stack-overflow. var nodeStack = s_nodeStackPool.Allocate(); nodeStack.Push(member); diff --git a/src/Compilers/Core/Portable/Syntax/SyntaxTree.cs b/src/Compilers/Core/Portable/Syntax/SyntaxTree.cs index f4e25ff87614f..a874a5a195f15 100644 --- a/src/Compilers/Core/Portable/Syntax/SyntaxTree.cs +++ b/src/Compilers/Core/Portable/Syntax/SyntaxTree.cs @@ -433,7 +433,7 @@ internal SourceGeneratorSyntaxTreeInfo GetSourceGeneratorInfo( if (syntaxHelper.ContainsGlobalAliases(root)) result |= SourceGeneratorSyntaxTreeInfo.ContainsGlobalAliases; - if (syntaxHelper.ContainsAttributeList(root)) + if (root.Green.Flags.HasFlag(GreenNode.NodeFlags.ContainsAttributes)) result |= SourceGeneratorSyntaxTreeInfo.ContainsAttributeList; _sourceGeneratorInfo = result; diff --git a/src/Compilers/VisualBasic/Portable/SourceGeneration/VisualBasicSyntaxHelper.vb b/src/Compilers/VisualBasic/Portable/SourceGeneration/VisualBasicSyntaxHelper.vb index 6d3572dbb55f0..80f5312759192 100644 --- a/src/Compilers/VisualBasic/Portable/SourceGeneration/VisualBasicSyntaxHelper.vb +++ b/src/Compilers/VisualBasic/Portable/SourceGeneration/VisualBasicSyntaxHelper.vb @@ -2,11 +2,7 @@ ' The .NET Foundation licenses this file to you under the MIT license. ' See the LICENSE file in the project root for more information. -Imports System.Runtime.CompilerServices -Imports System.Runtime.InteropServices -Imports System.Threading Imports Microsoft.CodeAnalysis.PooledObjects -Imports Microsoft.CodeAnalysis.SourceGeneration Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Namespace Microsoft.CodeAnalysis.VisualBasic @@ -20,8 +16,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Public Overrides ReadOnly Property IsCaseSensitive As Boolean = False - Protected Overrides ReadOnly Property AttributeListKind As Integer = SyntaxKind.AttributeList - Public Overrides Function IsValidIdentifier(name As String) As Boolean Return SyntaxFacts.IsValidIdentifier(name) End Function diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs b/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs index b447015006eab..72b7aea5b8d7a 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs +++ b/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs @@ -586,6 +586,7 @@ private void WriteGreenFactory(Node nd, bool withSyntaxFactoryContext = false) if (nd.Name != "SkippedTokensTriviaSyntax" && nd.Name != "DocumentationCommentTriviaSyntax" && nd.Name != "IncompleteMemberSyntax" && + nd.Name != "AttributeSyntax" && valueFields.Count + nodeFields.Count <= 3) { //int hash; diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb index ee8f0b071d620..12f1c7544afda 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb +++ b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeWriter.vb @@ -109,9 +109,9 @@ Friend Class GreenNodeWriter GenerateNodeStructureMembers(nodeStructure) ' Create the constructor. - GenerateNodeStructureConstructor(nodeStructure, False, noExtra:=True) - GenerateNodeStructureConstructor(nodeStructure, False, noExtra:=True, contextual:=True) - GenerateNodeStructureConstructor(nodeStructure, False) + GenerateNodeStructureConstructor(nodeStructure, noExtra:=True) + GenerateNodeStructureConstructor(nodeStructure, noExtra:=True, contextual:=True) + GenerateNodeStructureConstructor(nodeStructure) GenerateCreateRed(nodeStructure) @@ -293,7 +293,6 @@ Friend Class GreenNodeWriter ' Generate constructor for a node structure Private Sub GenerateNodeStructureConstructor(nodeStructure As ParseNodeStructure, - isRaw As Boolean, Optional noExtra As Boolean = False, Optional contextual As Boolean = False) From 7658b222ebf64a42bb423eec2670521889059df4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:50:10 -0800 Subject: [PATCH 13/26] Update vb --- .../Portable/Generated/Syntax.xml.Internal.Generated.vb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Compilers/VisualBasic/Portable/Generated/Syntax.xml.Internal.Generated.vb b/src/Compilers/VisualBasic/Portable/Generated/Syntax.xml.Internal.Generated.vb index 611c758ee9a67..ae104b19b7b84 100644 --- a/src/Compilers/VisualBasic/Portable/Generated/Syntax.xml.Internal.Generated.vb +++ b/src/Compilers/VisualBasic/Portable/Generated/Syntax.xml.Internal.Generated.vb @@ -8178,6 +8178,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Syntax.InternalSyntax Me._argumentList = argumentList End If + SetFlags(NodeFlags.ContainsAttributes) End Sub Friend Sub New(ByVal kind As SyntaxKind, target As AttributeTargetSyntax, name As TypeSyntax, argumentList As ArgumentListSyntax, context As ISyntaxFactoryContext) @@ -8196,6 +8197,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Syntax.InternalSyntax Me._argumentList = argumentList End If + SetFlags(NodeFlags.ContainsAttributes) End Sub Friend Sub New(ByVal kind As SyntaxKind, ByVal errors as DiagnosticInfo(), ByVal annotations as SyntaxAnnotation(), target As AttributeTargetSyntax, name As TypeSyntax, argumentList As ArgumentListSyntax) @@ -8213,6 +8215,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Syntax.InternalSyntax Me._argumentList = argumentList End If + SetFlags(NodeFlags.ContainsAttributes) End Sub Friend Overrides Function CreateRed(ByVal parent As SyntaxNode, ByVal startLocation As Integer) As SyntaxNode From 71d1c606b6f8a45c70c15f44aecc47f6582eb017 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:53:50 -0800 Subject: [PATCH 14/26] fix --- .../Syntax.xml.Internal.Generated.vb | 28 ++----------------- .../GreenNodes/GreenNodeFactoryWriter.vb | 1 + 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/src/Compilers/VisualBasic/Portable/Generated/Syntax.xml.Internal.Generated.vb b/src/Compilers/VisualBasic/Portable/Generated/Syntax.xml.Internal.Generated.vb index ae104b19b7b84..9473afa77c362 100644 --- a/src/Compilers/VisualBasic/Portable/Generated/Syntax.xml.Internal.Generated.vb +++ b/src/Compilers/VisualBasic/Portable/Generated/Syntax.xml.Internal.Generated.vb @@ -37514,19 +37514,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Syntax.InternalSyntax ''' Friend Shared Function Attribute(target As AttributeTargetSyntax, name As TypeSyntax, argumentList As ArgumentListSyntax) As AttributeSyntax Debug.Assert(name IsNot Nothing) - - Dim hash As Integer - Dim cached = SyntaxNodeCache.TryGetNode(SyntaxKind.Attribute, target, name, argumentList, hash) - If cached IsNot Nothing Then - Return DirectCast(cached, AttributeSyntax) - End If - - Dim result = New AttributeSyntax(SyntaxKind.Attribute, target, name, argumentList) - If hash >= 0 Then - SyntaxNodeCache.AddNode(result, hash) - End If - - Return result + Return New AttributeSyntax(SyntaxKind.Attribute, target, name, argumentList) End Function @@ -49592,19 +49580,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Syntax.InternalSyntax ''' Friend Function Attribute(target As AttributeTargetSyntax, name As TypeSyntax, argumentList As ArgumentListSyntax) As AttributeSyntax Debug.Assert(name IsNot Nothing) - - Dim hash As Integer - Dim cached = VisualBasicSyntaxNodeCache.TryGetNode(SyntaxKind.Attribute, target, name, argumentList, _factoryContext, hash) - If cached IsNot Nothing Then - Return DirectCast(cached, AttributeSyntax) - End If - - Dim result = New AttributeSyntax(SyntaxKind.Attribute, target, name, argumentList, _factoryContext) - If hash >= 0 Then - SyntaxNodeCache.AddNode(result, hash) - End If - - Return result + Return New AttributeSyntax(SyntaxKind.Attribute, target, name, argumentList, _factoryContext) End Function diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeFactoryWriter.vb b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeFactoryWriter.vb index 801c76f8aadec..43115c95069c7 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeFactoryWriter.vb +++ b/src/Tools/Source/CompilerGeneratorTools/Source/VisualBasicSyntaxGenerator/GreenNodes/GreenNodeFactoryWriter.vb @@ -264,6 +264,7 @@ Friend Class GreenNodeFactoryWriter nodeStructure.Name = "SkippedTokensTriviaSyntax" OrElse nodeStructure.Name = "DocumentationCommentTriviaSyntax" OrElse nodeStructure.Name.EndsWith("DirectiveTriviaSyntax", StringComparison.Ordinal) OrElse + nodeStructure.Name = "AttributeSyntax" OrElse allFields.Count + allChildren.Count > 3) Then _writer.Write(" Return New {0}(", StructureTypeName(nodeStructure)) From 7fb9f8e7637b63f1e02642d7bee95edc3459a59e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:57:22 -0800 Subject: [PATCH 15/26] extract type --- .../Syntax/GreenNode.NodeFlagsAndSlotCount.cs | 74 +++++++++++++++++++ .../Core/Portable/Syntax/GreenNode.cs | 63 ---------------- 2 files changed, 74 insertions(+), 63 deletions(-) create mode 100644 src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs new file mode 100644 index 0000000000000..b964dc3612d64 --- /dev/null +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs @@ -0,0 +1,74 @@ +// 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.Diagnostics; + +namespace Microsoft.CodeAnalysis +{ + internal abstract partial class GreenNode + { + /// + /// Combination of and stored in a single 16bit value. + /// + private struct NodeFlagsAndSlotCount + { + /// + /// 4 bits for the SlotCount. This allows slot counts of 0-14 to be stored as a direct byte. All 1s + /// indicates that the slot count must be computed. + /// + private const ushort SlotCountMask = 0b1111000000000000; + private const ushort NodeFlagsMask = 0b0000111111111111; + + private const int SlotCountShift = 12; + private const int MaxSlotCount = 15; + + /// + /// 12 bits for the NodeFlags. This allows for up to 12 distinct bits to be stored to designate interesting + /// aspects of a node. + /// + + /// + /// CCCCFFFFFFFFFFFF for Count bits then Flag bits. + /// + private ushort _data; + + public byte SlotCount + { + readonly get + { + var shifted = _data >> SlotCountShift; + Debug.Assert(shifted <= MaxSlotCount); + var result = (byte)shifted; + return result == MaxSlotCount ? byte.MaxValue : result; + } + + set + { + if (value >= MaxSlotCount) + value = MaxSlotCount; + Debug.Assert(value <= MaxSlotCount); + + // Clear out everything but the node-flags, and then assign into the slot-count segment. + _data = (ushort)((_data & NodeFlagsMask) | (value << SlotCountShift)); + } + } + + public NodeFlags NodeFlags + { + readonly get + { + return (NodeFlags)(_data & NodeFlagsMask); + } + + set + { + Debug.Assert((ushort)value <= NodeFlagsMask); + + // Clear out everything but the slot-count, and then assign into the node-flags segment. + _data = (ushort)((_data & SlotCountMask) | (ushort)value); + } + } + } + } +} diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 384f02a8f936e..b12845b6a7891 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -19,69 +19,6 @@ namespace Microsoft.CodeAnalysis [DebuggerDisplay("{GetDebuggerDisplay(), nq}")] internal abstract partial class GreenNode { - /// - /// Combination of and stored in a single 16bit value. - /// - internal struct NodeFlagsAndSlotCount - { - /// - /// 4 bits for the SlotCount. This allows slot counts of 0-14 to be stored as a direct byte. All 1s - /// indicates that the slot count must be computed. - /// - private const ushort SlotCountMask = 0b1111000000000000; - private const ushort NodeFlagsMask = 0b0000111111111111; - - private const int SlotCountShift = 12; - private const int MaxSlotCount = 15; - - /// - /// 12 bits for the NodeFlags. This allows for up to 12 distinct bits to be stored to designate interesting - /// aspects of a node. - /// - - /// - /// CCCCFFFFFFFFFFFF for Count bits then Flag bits. - /// - private ushort _data; - - public byte SlotCount - { - readonly get - { - var shifted = _data >> SlotCountShift; - Debug.Assert(shifted <= MaxSlotCount); - var result = (byte)shifted; - return result == MaxSlotCount ? byte.MaxValue : result; - } - - set - { - if (value >= MaxSlotCount) - value = MaxSlotCount; - Debug.Assert(value <= MaxSlotCount); - - // Clear out everything but the node-flags, and then assign into the slot-count segment. - _data = (ushort)((_data & NodeFlagsMask) | (value << SlotCountShift)); - } - } - - public NodeFlags NodeFlags - { - readonly get - { - return (NodeFlags)(_data & NodeFlagsMask); - } - - set - { - Debug.Assert((ushort)value <= NodeFlagsMask); - - // Clear out everything but the slot-count, and then assign into the node-flags segment. - _data = (ushort)((_data & SlotCountMask) | (ushort)value); - } - } - } - private string GetDebuggerDisplay() { return this.GetType().Name + " " + this.KindText + " " + this.ToString(); From ccd891a91895c476009840933e31b2c481934160 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 10:59:02 -0800 Subject: [PATCH 16/26] Update src/Compilers/Core/Portable/Syntax/GreenNode.cs --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index b12845b6a7891..32f135f6c51ae 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -15,7 +15,6 @@ namespace Microsoft.CodeAnalysis { - [DebuggerDisplay("{GetDebuggerDisplay(), nq}")] internal abstract partial class GreenNode { From bef9b626b34dd375a30e30f5922d98d3e374c944 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 11:05:00 -0800 Subject: [PATCH 17/26] Move into loop --- .../SyntaxValueProvider_ForAttributeWithSimpleName.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs index ae6152f2dd267..b496863637e0d 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs @@ -234,10 +234,6 @@ void processMember(SyntaxNode member) { cancellationToken.ThrowIfCancellationRequested(); - // Don't bother descending into nodes that don't contain attributes. - if (!member.Green.Flags.HasFlag(GreenNode.NodeFlags.ContainsAttributes)) - return; - // nodes can be arbitrarily deep. Use an explicit stack over recursion to prevent a stack-overflow. var nodeStack = s_nodeStackPool.Allocate(); nodeStack.Push(member); @@ -248,6 +244,10 @@ void processMember(SyntaxNode member) { var node = nodeStack.Pop(); + // Don't bother descending into nodes that don't contain attributes. + if (!node.Green.Flags.HasFlag(GreenNode.NodeFlags.ContainsAttributes)) + continue; + if (syntaxHelper.IsAttributeList(node)) { foreach (var attribute in syntaxHelper.GetAttributesOfAttributeList(node)) From 7890a62d7c2ce81f05bb30f22f3346b258934505 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 11:05:41 -0800 Subject: [PATCH 18/26] Keep both checks --- .../Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs index b496863637e0d..8644ed54c9666 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs @@ -234,6 +234,10 @@ void processMember(SyntaxNode member) { cancellationToken.ThrowIfCancellationRequested(); + // Don't bother descending into nodes that don't contain attributes. + if (!member.Green.Flags.HasFlag(GreenNode.NodeFlags.ContainsAttributes)) + return; + // nodes can be arbitrarily deep. Use an explicit stack over recursion to prevent a stack-overflow. var nodeStack = s_nodeStackPool.Allocate(); nodeStack.Push(member); From 5f23cb52d11d65f2a0434f3a10990961c4d1cd96 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 11:07:28 -0800 Subject: [PATCH 19/26] Simplify --- .../SyntaxValueProvider_ForAttributeWithSimpleName.cs | 4 ++-- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 8 ++++++++ src/Compilers/Core/Portable/Syntax/SyntaxNode.cs | 2 ++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs index 8644ed54c9666..f2fa22112ded2 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxValueProvider_ForAttributeWithSimpleName.cs @@ -235,7 +235,7 @@ void processMember(SyntaxNode member) cancellationToken.ThrowIfCancellationRequested(); // Don't bother descending into nodes that don't contain attributes. - if (!member.Green.Flags.HasFlag(GreenNode.NodeFlags.ContainsAttributes)) + if (!member.ContainsAttributes) return; // nodes can be arbitrarily deep. Use an explicit stack over recursion to prevent a stack-overflow. @@ -249,7 +249,7 @@ void processMember(SyntaxNode member) var node = nodeStack.Pop(); // Don't bother descending into nodes that don't contain attributes. - if (!node.Green.Flags.HasFlag(GreenNode.NodeFlags.ContainsAttributes)) + if (!node.ContainsAttributes) continue; if (syntaxHelper.IsAttributeList(node)) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 32f135f6c51ae..e547b38257cbd 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -341,6 +341,14 @@ public bool ContainsDirectives } } + public bool ContainsAttributes + { + get + { + return (this.flags & NodeFlags.ContainsAttributes) != 0; + } + } + public bool ContainsDiagnostics { get diff --git a/src/Compilers/Core/Portable/Syntax/SyntaxNode.cs b/src/Compilers/Core/Portable/Syntax/SyntaxNode.cs index 9f77eb52eeb95..ba5ab7ed1b3dc 100644 --- a/src/Compilers/Core/Portable/Syntax/SyntaxNode.cs +++ b/src/Compilers/Core/Portable/Syntax/SyntaxNode.cs @@ -442,6 +442,8 @@ public bool ContainsDiagnostics /// public bool ContainsDirectives => this.Green.ContainsDirectives; + internal bool ContainsAttributes => this.Green.ContainsAttributes; + /// /// Returns true if this node contains any directives (e.g. #if, #nullable, etc.) within it with a matching kind. /// From 8f2bc752321dc9eb8f028f0b6acc704f05589710 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 11:07:57 -0800 Subject: [PATCH 20/26] Simplify --- src/Compilers/Core/Portable/Syntax/SyntaxTree.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/Syntax/SyntaxTree.cs b/src/Compilers/Core/Portable/Syntax/SyntaxTree.cs index a874a5a195f15..6e9655d6fcf14 100644 --- a/src/Compilers/Core/Portable/Syntax/SyntaxTree.cs +++ b/src/Compilers/Core/Portable/Syntax/SyntaxTree.cs @@ -433,7 +433,7 @@ internal SourceGeneratorSyntaxTreeInfo GetSourceGeneratorInfo( if (syntaxHelper.ContainsGlobalAliases(root)) result |= SourceGeneratorSyntaxTreeInfo.ContainsGlobalAliases; - if (root.Green.Flags.HasFlag(GreenNode.NodeFlags.ContainsAttributes)) + if (root.ContainsAttributes) result |= SourceGeneratorSyntaxTreeInfo.ContainsAttributeList; _sourceGeneratorInfo = result; From afe9ad1f2542114da0e81f7547739b34be20a891 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 11:24:51 -0800 Subject: [PATCH 21/26] docs --- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index e547b38257cbd..54b3596801a9d 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -25,6 +25,10 @@ private string GetDebuggerDisplay() internal const int ListKind = 1; + // Pack the kind, node-flags, slot-count, and full-width into 64bits. Note: if we need more bits in the future + // (say for additional node-flags), we can always directly use a packed int64 here, and manage where all these + // bits go manually. + private readonly ushort _kind; private NodeFlagsAndSlotCount _nodeFlagsAndSlotCount; private int _fullWidth; From 4ca709addb17d64524d1f429beb5a5c101f07d6e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 11:40:12 -0800 Subject: [PATCH 22/26] Setflags --- .../Source/CSharpSyntaxGenerator/SourceWriter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs b/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs index 72b7aea5b8d7a..4a999bf7f0d92 100644 --- a/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs +++ b/src/Tools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/SourceWriter.cs @@ -293,7 +293,7 @@ private void WriteCtorBody(Node node, List valueFields, List nodeF { if (node.Name == "AttributeSyntax") { - WriteLine("this.flags |= NodeFlags.ContainsAttributes;"); + WriteLine("SetFlags(NodeFlags.ContainsAttributes);"); } // constructor body From 9ef94fc369e6100baf51c94c4a8b6458ef0d7ca6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 13 Feb 2024 11:41:12 -0800 Subject: [PATCH 23/26] update code --- .../Syntax.xml.Internal.Generated.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs b/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs index 640015f6944e8..8b0800ab52f00 100644 --- a/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs +++ b/src/Compilers/CSharp/Portable/Generated/CSharpSyntaxGenerator/CSharpSyntaxGenerator.SourceGenerator/Syntax.xml.Internal.Generated.cs @@ -15864,7 +15864,7 @@ internal sealed partial class AttributeSyntax : CSharpSyntaxNode internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentListSyntax? argumentList, DiagnosticInfo[]? diagnostics, SyntaxAnnotation[]? annotations) : base(kind, diagnostics, annotations) { - this.flags |= NodeFlags.ContainsAttributes; + SetFlags(NodeFlags.ContainsAttributes); this.SlotCount = 2; this.AdjustFlagsAndWidth(name); this.name = name; @@ -15879,7 +15879,7 @@ internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentList : base(kind) { this.SetFactoryContext(context); - this.flags |= NodeFlags.ContainsAttributes; + SetFlags(NodeFlags.ContainsAttributes); this.SlotCount = 2; this.AdjustFlagsAndWidth(name); this.name = name; @@ -15893,7 +15893,7 @@ internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentList internal AttributeSyntax(SyntaxKind kind, NameSyntax name, AttributeArgumentListSyntax? argumentList) : base(kind) { - this.flags |= NodeFlags.ContainsAttributes; + SetFlags(NodeFlags.ContainsAttributes); this.SlotCount = 2; this.AdjustFlagsAndWidth(name); this.name = name; From fa6c04d262c0faebe0c23dba1e5d5aa34b559f42 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 14 Feb 2024 16:31:31 -0800 Subject: [PATCH 24/26] Add special constant --- .../Syntax/GreenNode.NodeFlagsAndSlotCount.cs | 24 ++++++++++++------- .../Core/Portable/Syntax/GreenNode.cs | 7 +++--- .../SyntaxList.WithManyChildren.cs | 15 ++++++------ 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs index b964dc3612d64..47d4763675934 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs @@ -11,7 +11,7 @@ internal abstract partial class GreenNode /// /// Combination of and stored in a single 16bit value. /// - private struct NodeFlagsAndSlotCount + protected struct NodeFlagsAndSlotCount { /// /// 4 bits for the SlotCount. This allows slot counts of 0-14 to be stored as a direct byte. All 1s @@ -21,7 +21,12 @@ private struct NodeFlagsAndSlotCount private const ushort NodeFlagsMask = 0b0000111111111111; private const int SlotCountShift = 12; - private const int MaxSlotCount = 15; + + /// + /// Value used to indicate the slot count was too large to be encoded directly in our + /// value. Callers will have to store the value elsewhere and retrieve the full value themselves. + /// + public const int SlotCountTooLarge = 0b0000000000001111; /// /// 12 bits for the NodeFlags. This allows for up to 12 distinct bits to be stored to designate interesting @@ -33,21 +38,22 @@ private struct NodeFlagsAndSlotCount /// private ushort _data; - public byte SlotCount + /// + /// + /// + public byte SmallSlotCount { readonly get { var shifted = _data >> SlotCountShift; - Debug.Assert(shifted <= MaxSlotCount); - var result = (byte)shifted; - return result == MaxSlotCount ? byte.MaxValue : result; + Debug.Assert(shifted <= SlotCountTooLarge); + return (byte)shifted; } set { - if (value >= MaxSlotCount) - value = MaxSlotCount; - Debug.Assert(value <= MaxSlotCount); + if (value > SlotCountTooLarge) + value = SlotCountTooLarge; // Clear out everything but the node-flags, and then assign into the slot-count segment. _data = (ushort)((_data & NodeFlagsMask) | (value << SlotCountShift)); diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index 54b3596801a9d..b008301d51de0 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -39,10 +39,11 @@ protected NodeFlags flags set => _nodeFlagsAndSlotCount.NodeFlags = value; } + /// > private byte _slotCount { - get => _nodeFlagsAndSlotCount.SlotCount; - set => _nodeFlagsAndSlotCount.SlotCount = value; + get => _nodeFlagsAndSlotCount.SmallSlotCount; + set => _nodeFlagsAndSlotCount.SmallSlotCount = value; } private static readonly ConditionalWeakTable s_diagnosticsTable = @@ -157,7 +158,7 @@ public int SlotCount get { int count = _slotCount; - if (count == byte.MaxValue) + if (count == NodeFlagsAndSlotCount.SlotCountTooLarge) { count = GetSlotCount(); } diff --git a/src/Compilers/Core/Portable/Syntax/InternalSyntax/SyntaxList.WithManyChildren.cs b/src/Compilers/Core/Portable/Syntax/InternalSyntax/SyntaxList.WithManyChildren.cs index 503d907700cec..89127a9e9bbfb 100644 --- a/src/Compilers/Core/Portable/Syntax/InternalSyntax/SyntaxList.WithManyChildren.cs +++ b/src/Compilers/Core/Portable/Syntax/InternalSyntax/SyntaxList.WithManyChildren.cs @@ -29,14 +29,13 @@ internal WithManyChildrenBase(DiagnosticInfo[]? diagnostics, SyntaxAnnotation[]? private void InitializeChildren() { int n = children.Length; - if (n < byte.MaxValue) - { - this.SlotCount = (byte)n; - } - else - { - this.SlotCount = byte.MaxValue; - } + + // Attempt to store small lengths directly into the storage provided within GreenNode. If, however, the + // length is too long, we will store a special value in the space, which will `SlotCount` to call back + // into `GetSlotCount` to retrieve the true length. + this.SlotCount = n < NodeFlagsAndSlotCount.SlotCountTooLarge + ? (byte)n + : NodeFlagsAndSlotCount.SlotCountTooLarge; for (int i = 0; i < children.Length; i++) { From abe46cb5457ce1ac61e0b5f0c061c93322bbc51b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 14 Feb 2024 16:32:36 -0800 Subject: [PATCH 25/26] Docs --- .../Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs index 47d4763675934..f75f37b276a75 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs @@ -39,7 +39,8 @@ protected struct NodeFlagsAndSlotCount private ushort _data; /// - /// + /// Returns the slot count if it was small enough to be stored directly in this object. Otherwise, returns + /// to indicate it could not be directly stored. /// public byte SmallSlotCount { From e597ad1740c59bd94bfd274c50cbb4c754cdd0f8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 14 Feb 2024 16:34:14 -0800 Subject: [PATCH 26/26] Pull out constant --- .../Syntax/GreenNode.NodeFlagsAndSlotCount.cs | 8 +------- src/Compilers/Core/Portable/Syntax/GreenNode.cs | 13 +++++++------ .../InternalSyntax/SyntaxList.WithManyChildren.cs | 4 ++-- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs index f75f37b276a75..f3249a121238c 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.NodeFlagsAndSlotCount.cs @@ -11,7 +11,7 @@ internal abstract partial class GreenNode /// /// Combination of and stored in a single 16bit value. /// - protected struct NodeFlagsAndSlotCount + private struct NodeFlagsAndSlotCount { /// /// 4 bits for the SlotCount. This allows slot counts of 0-14 to be stored as a direct byte. All 1s @@ -22,12 +22,6 @@ protected struct NodeFlagsAndSlotCount private const int SlotCountShift = 12; - /// - /// Value used to indicate the slot count was too large to be encoded directly in our - /// value. Callers will have to store the value elsewhere and retrieve the full value themselves. - /// - public const int SlotCountTooLarge = 0b0000000000001111; - /// /// 12 bits for the NodeFlags. This allows for up to 12 distinct bits to be stored to designate interesting /// aspects of a node. diff --git a/src/Compilers/Core/Portable/Syntax/GreenNode.cs b/src/Compilers/Core/Portable/Syntax/GreenNode.cs index b008301d51de0..ce0d2d07a96ed 100644 --- a/src/Compilers/Core/Portable/Syntax/GreenNode.cs +++ b/src/Compilers/Core/Portable/Syntax/GreenNode.cs @@ -29,6 +29,12 @@ private string GetDebuggerDisplay() // (say for additional node-flags), we can always directly use a packed int64 here, and manage where all these // bits go manually. + /// + /// Value used to indicate the slot count was too large to be encoded directly in our + /// value. Callers will have to store the value elsewhere and retrieve the full value themselves. + /// + protected const int SlotCountTooLarge = 0b0000000000001111; + private readonly ushort _kind; private NodeFlagsAndSlotCount _nodeFlagsAndSlotCount; private int _fullWidth; @@ -158,12 +164,7 @@ public int SlotCount get { int count = _slotCount; - if (count == NodeFlagsAndSlotCount.SlotCountTooLarge) - { - count = GetSlotCount(); - } - - return count; + return count == SlotCountTooLarge ? GetSlotCount() : count; } protected set diff --git a/src/Compilers/Core/Portable/Syntax/InternalSyntax/SyntaxList.WithManyChildren.cs b/src/Compilers/Core/Portable/Syntax/InternalSyntax/SyntaxList.WithManyChildren.cs index 89127a9e9bbfb..842d3e8f3a727 100644 --- a/src/Compilers/Core/Portable/Syntax/InternalSyntax/SyntaxList.WithManyChildren.cs +++ b/src/Compilers/Core/Portable/Syntax/InternalSyntax/SyntaxList.WithManyChildren.cs @@ -33,9 +33,9 @@ private void InitializeChildren() // Attempt to store small lengths directly into the storage provided within GreenNode. If, however, the // length is too long, we will store a special value in the space, which will `SlotCount` to call back // into `GetSlotCount` to retrieve the true length. - this.SlotCount = n < NodeFlagsAndSlotCount.SlotCountTooLarge + this.SlotCount = n < SlotCountTooLarge ? (byte)n - : NodeFlagsAndSlotCount.SlotCountTooLarge; + : SlotCountTooLarge; for (int i = 0; i < children.Length; i++) {