From 3799cae7920f29dd5a2331c4b2438b5d27f035a6 Mon Sep 17 00:00:00 2001 From: Matt Warren Date: Fri, 20 Feb 2015 13:03:46 -0800 Subject: [PATCH 1/2] Fix calculation for slot containing offset Fixes issue #605 Add logic to not return zero-length nodes, since they never contain their offset. --- .../SyntaxList.WithLotsOfChildren.cs | 14 ++++++- .../Test/Syntax/Syntax/SyntaxNodeTests.cs | 38 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Syntax/InternalSyntax/SyntaxList.WithLotsOfChildren.cs b/src/Compilers/CSharp/Portable/Syntax/InternalSyntax/SyntaxList.WithLotsOfChildren.cs index b2c084dc326cb..cef45394e21ad 100644 --- a/src/Compilers/CSharp/Portable/Syntax/InternalSyntax/SyntaxList.WithLotsOfChildren.cs +++ b/src/Compilers/CSharp/Portable/Syntax/InternalSyntax/SyntaxList.WithLotsOfChildren.cs @@ -53,7 +53,19 @@ public override int FindSlotIndexContainingOffset(int offset) { Debug.Assert(offset >= 0 && offset < FullWidth); int idx = _childOffsets.BinarySearch(offset); - return idx >= 0 ? idx : (~idx - 1); + + if (idx < 0) + { + idx = (~idx - 1); + } + + // skip zero-length nodes (they won't ever contain the offset) + while (idx < _childOffsets.Length - 1 && _childOffsets[idx] == _childOffsets[idx + 1]) + { + idx++; + } + + return idx; } private static int[] CalculateOffsets(ArrayElement[] children) diff --git a/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs b/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs index bf0831ed87b6b..c1b5a0c2b571e 100644 --- a/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs @@ -481,6 +481,44 @@ public void TestFindToken() Assert.Equal(SyntaxKind.IfKeyword, token.Kind()); } + [Fact] + public void TestFindTokenInLargeList() + { + var identifier = SyntaxFactory.Identifier("x"); + var missingIdentifier = SyntaxFactory.MissingToken(SyntaxKind.IdentifierToken); + var name = SyntaxFactory.IdentifierName(identifier); + var missingName = SyntaxFactory.IdentifierName(missingIdentifier); + var comma = SyntaxFactory.Token(SyntaxKind.CommaToken); + var missingComma = SyntaxFactory.MissingToken(SyntaxKind.CommaToken); + var argument = SyntaxFactory.Argument(name); + var missingArgument = SyntaxFactory.Argument(missingName); + + // make a large list that has lots of zero-length nodes (that shouldn't be found) + var nodesAndTokens = SyntaxFactory.NodeOrTokenList( + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + argument); + + var argumentList = SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(SyntaxFactory.NodeOrTokenList(nodesAndTokens))); + var invocation = SyntaxFactory.InvocationExpression(name, argumentList); + CheckFindToken(invocation); + } + + private void CheckFindToken(SyntaxNode node) + { + for (int i = node.FullSpan.End - 1; i >= 0; i--) + { + var token = node.FindToken(i); + Assert.Equal(true, token.FullSpan.Contains(i)); + } + } + [WorkItem(755236, "DevDiv")] [Fact] public void TestFindNode() From b106d3d40e93b739f2378398ec8f06fe7154e16d Mon Sep 17 00:00:00 2001 From: Matt Warren Date: Mon, 23 Feb 2015 10:49:57 -0800 Subject: [PATCH 2/2] Change Slot lookup to use BinarySearchUpperBound --- .../SyntaxList.WithLotsOfChildren.cs | 15 +------- .../Test/Syntax/Syntax/SyntaxNodeTests.cs | 2 +- .../InternalUtilities/ArrayExtensions.cs | 30 ++++++++++++++++ .../Syntax/InternalSyntax/SyntaxList.vb | 3 +- .../Test/Syntax/TestSyntaxNodes.vb | 35 +++++++++++++++++++ 5 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Syntax/InternalSyntax/SyntaxList.WithLotsOfChildren.cs b/src/Compilers/CSharp/Portable/Syntax/InternalSyntax/SyntaxList.WithLotsOfChildren.cs index cef45394e21ad..186bb5cea2758 100644 --- a/src/Compilers/CSharp/Portable/Syntax/InternalSyntax/SyntaxList.WithLotsOfChildren.cs +++ b/src/Compilers/CSharp/Portable/Syntax/InternalSyntax/SyntaxList.WithLotsOfChildren.cs @@ -52,20 +52,7 @@ public override int GetSlotOffset(int index) public override int FindSlotIndexContainingOffset(int offset) { Debug.Assert(offset >= 0 && offset < FullWidth); - int idx = _childOffsets.BinarySearch(offset); - - if (idx < 0) - { - idx = (~idx - 1); - } - - // skip zero-length nodes (they won't ever contain the offset) - while (idx < _childOffsets.Length - 1 && _childOffsets[idx] == _childOffsets[idx + 1]) - { - idx++; - } - - return idx; + return _childOffsets.BinarySearchUpperBound(offset) - 1; } private static int[] CalculateOffsets(ArrayElement[] children) diff --git a/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs b/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs index c1b5a0c2b571e..689ba85c8b4a8 100644 --- a/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs @@ -512,7 +512,7 @@ public void TestFindTokenInLargeList() private void CheckFindToken(SyntaxNode node) { - for (int i = node.FullSpan.End - 1; i >= 0; i--) + for (int i = 0; i < node.FullSpan.End; i++) { var token = node.FindToken(i); Assert.Equal(true, token.FullSpan.Contains(i)); diff --git a/src/Compilers/Core/Portable/InternalUtilities/ArrayExtensions.cs b/src/Compilers/Core/Portable/InternalUtilities/ArrayExtensions.cs index 54ad0fe639c8d..819e149fdbc64 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/ArrayExtensions.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/ArrayExtensions.cs @@ -173,5 +173,35 @@ internal static int BinarySearch(this int[] array, int value) return ~low; } + + /// + /// Search a sorted integer array for the target value in O(log N) time. + /// + /// The array of integers which must be sorted in ascending order. + /// The target value. + /// An index in the array pointing to the position where should be + /// inserted in order to maintain the sorted order. All values to the right of this position will be + /// strictly greater than . Note that this may return a position off the end + /// of the array if all elements are less than or equal to . + internal static int BinarySearchUpperBound(this int[] array, int value) + { + int low = 0; + int high = array.Length - 1; + + while (low <= high) + { + int middle = low + ((high - low) >> 1); + if (array[middle] > value) + { + high = middle - 1; + } + else + { + low = middle + 1; + } + } + + return low; + } } } diff --git a/src/Compilers/VisualBasic/Portable/Syntax/InternalSyntax/SyntaxList.vb b/src/Compilers/VisualBasic/Portable/Syntax/InternalSyntax/SyntaxList.vb index cccfb125a203d..9447aca14b3bd 100644 --- a/src/Compilers/VisualBasic/Portable/Syntax/InternalSyntax/SyntaxList.vb +++ b/src/Compilers/VisualBasic/Portable/Syntax/InternalSyntax/SyntaxList.vb @@ -466,8 +466,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Syntax.InternalSyntax ''' Public Overrides Function FindSlotIndexContainingOffset(offset As Integer) As Integer Debug.Assert(offset >= 0 AndAlso offset < FullWidth) - Dim idx = _childOffsets.BinarySearch(offset) - Return If(idx >= 0, idx, (Not idx) - 1) + Return _childOffsets.BinarySearchUpperBound(offset) - 1 End Function Friend Overrides Function SetDiagnostics(errors() As DiagnosticInfo) As GreenNode diff --git a/src/Compilers/VisualBasic/Test/Syntax/TestSyntaxNodes.vb b/src/Compilers/VisualBasic/Test/Syntax/TestSyntaxNodes.vb index de391113531f3..b9858f0e700ca 100644 --- a/src/Compilers/VisualBasic/Test/Syntax/TestSyntaxNodes.vb +++ b/src/Compilers/VisualBasic/Test/Syntax/TestSyntaxNodes.vb @@ -2114,6 +2114,41 @@ End Class]]> Assert.Throws(Of ArgumentOutOfRangeException)(Sub() classDecl.FindNode(root.FullSpan)) End Sub + + Public Sub TestFindTokenInLargeList() + Dim identifier = SyntaxFactory.Identifier("x") + Dim missingIdentifier = SyntaxFactory.MissingToken(SyntaxKind.IdentifierToken) + Dim name = SyntaxFactory.IdentifierName(identifier) + Dim missingName = SyntaxFactory.IdentifierName(missingIdentifier) + Dim comma = SyntaxFactory.Token(SyntaxKind.CommaToken) + Dim missingComma = SyntaxFactory.MissingToken(SyntaxKind.CommaToken) + Dim argument = SyntaxFactory.SimpleArgument(name) + Dim missingArgument = SyntaxFactory.SimpleArgument(missingName) + + '' make a large list that has lots of zero-length nodes (that shouldn't be found) + Dim nodesAndTokens = SyntaxFactory.NodeOrTokenList( + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + missingArgument, missingComma, + argument) + + Dim argumentList = SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(Of ArgumentSyntax)(SyntaxFactory.NodeOrTokenList(nodesAndTokens))) + Dim invocation = SyntaxFactory.InvocationExpression(name, argumentList) + CheckFindToken(invocation) + End Sub + + Private Sub CheckFindToken(node As SyntaxNode) + For i As Integer = 1 To node.FullSpan.End - 1 + Dim token = node.FindToken(i) + Assert.Equal(True, token.FullSpan.Contains(i)) + Next + End Sub + Public Sub TestFindTriviaNoTriviaExistsAtPosition()