From 9a80ccf7dbd3be1c89435c29a17e4984c9ba4d68 Mon Sep 17 00:00:00 2001 From: Viktor Hofer Date: Thu, 15 Feb 2018 23:32:19 +0100 Subject: [PATCH] Fix regression & enable ClearInitLocals in System.Text.RegularExpressions (dotnet/corefx#27146) * Fix bug in RegexWriter ref struct change * Remove duplicate ValueListBuilder implementation * Enable ILLinkClearInitLocals * Rename concrete ValueListBuilder impl to Pop Commit migrated from https://github.com/dotnet/corefx/commit/9e0151b48f3a45d497e8835423ed38e7661cd056 --- .../src/System.Text.RegularExpressions.csproj | 6 +- .../System/Text/RegularExpressions/RegexWriter.cs | 51 +++++++-------- .../ResizableValueListBuilder.cs | 76 ---------------------- .../RegularExpressions/ValueListBuilder.Pop.cs | 23 +++++++ 4 files changed, 53 insertions(+), 103 deletions(-) delete mode 100644 src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/ResizableValueListBuilder.cs create mode 100644 src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/ValueListBuilder.Pop.cs diff --git a/src/libraries/System.Text.RegularExpressions/src/System.Text.RegularExpressions.csproj b/src/libraries/System.Text.RegularExpressions/src/System.Text.RegularExpressions.csproj index 49efc8a..ca0f924 100644 --- a/src/libraries/System.Text.RegularExpressions/src/System.Text.RegularExpressions.csproj +++ b/src/libraries/System.Text.RegularExpressions/src/System.Text.RegularExpressions.csproj @@ -5,6 +5,7 @@ {2C58640B-5BED-4E83-9554-CD2B9762643F} System.Text.RegularExpressions $(DefineConstants);FEATURE_COMPILED + true @@ -33,7 +34,7 @@ - + @@ -43,6 +44,9 @@ Common\System\IO\StringBuilderCache.cs + + Common\System\Collections\Generic\ValueListBuilder.cs + diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexWriter.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexWriter.cs index d75c851..e1481ab 100644 --- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexWriter.cs +++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexWriter.cs @@ -26,8 +26,8 @@ namespace System.Text.RegularExpressions private const int EmittedSize = 56; private const int IntStackSize = 32; - private ResizableValueListBuilder _emitted; - private ResizableValueListBuilder _intStack; + private ValueListBuilder _emitted; + private ValueListBuilder _intStack; private readonly Dictionary _stringHash; private readonly List _stringTable; private Hashtable _caps; @@ -35,8 +35,8 @@ namespace System.Text.RegularExpressions private RegexWriter(Span emittedSpan, Span intStackSpan) { - _emitted = new ResizableValueListBuilder(emittedSpan); - _intStack = new ResizableValueListBuilder(intStackSpan); + _emitted = new ValueListBuilder(emittedSpan); + _intStack = new ValueListBuilder(intStackSpan); _stringHash = new Dictionary(); _stringTable = new List(); _caps = null; @@ -49,7 +49,10 @@ namespace System.Text.RegularExpressions /// internal static RegexCode Write(RegexTree t) { - RegexWriter w = new RegexWriter(); + Span emittedSpan = stackalloc int[EmittedSize]; + Span intStackSpan = stackalloc int[IntStackSize]; + var w = new RegexWriter(emittedSpan, intStackSpan); + RegexCode retval = w.RegexCodeFromRegexTree(t); #if DEBUG if (t.Debug) @@ -66,60 +69,56 @@ namespace System.Text.RegularExpressions /// through the tree and calls EmitFragment to emits code before /// and after each child of an interior node, and at each leaf. /// - public RegexCode RegexCodeFromRegexTree(RegexTree tree) + private RegexCode RegexCodeFromRegexTree(RegexTree tree) { - Span emittedSpan = stackalloc int[EmittedSize]; - Span intStackSpan = stackalloc int[IntStackSize]; - RegexWriter writer = new RegexWriter(emittedSpan, intStackSpan); - // construct sparse capnum mapping if some numbers are unused int capsize; if (tree._capnumlist == null || tree._captop == tree._capnumlist.Length) { capsize = tree._captop; - writer._caps = null; + _caps = null; } else { capsize = tree._capnumlist.Length; - writer._caps = tree._caps; + _caps = tree._caps; for (int i = 0; i < tree._capnumlist.Length; i++) - writer._caps[tree._capnumlist[i]] = i; + _caps[tree._capnumlist[i]] = i; } RegexNode curNode = tree._root; int curChild = 0; - writer.Emit(RegexCode.Lazybranch, 0); + Emit(RegexCode.Lazybranch, 0); for (; ; ) { if (curNode._children == null) { - writer.EmitFragment(curNode._type, curNode, 0); + EmitFragment(curNode._type, curNode, 0); } else if (curChild < curNode._children.Count) { - writer.EmitFragment(curNode._type | BeforeChild, curNode, curChild); + EmitFragment(curNode._type | BeforeChild, curNode, curChild); curNode = curNode._children[curChild]; - writer._intStack.Append(curChild); + _intStack.Append(curChild); curChild = 0; continue; } - if (writer._intStack.Length == 0) + if (_intStack.Length == 0) break; - curChild = writer._intStack.Pop(); + curChild = _intStack.Pop(); curNode = curNode._next; - writer.EmitFragment(curNode._type | AfterChild, curNode, curChild); + EmitFragment(curNode._type | AfterChild, curNode, curChild); curChild++; } - writer.PatchJump(0, writer._emitted.Length); - writer.Emit(RegexCode.Stop); + PatchJump(0, _emitted.Length); + Emit(RegexCode.Stop); RegexPrefix fcPrefix = RegexFCD.FirstChars(tree); RegexPrefix prefix = RegexFCD.Prefix(tree); @@ -134,13 +133,13 @@ namespace System.Text.RegularExpressions bmPrefix = null; int anchors = RegexFCD.Anchors(tree); - int[] emitted = writer._emitted.AsReadOnlySpan().ToArray(); + int[] emitted = _emitted.AsReadOnlySpan().ToArray(); // Cleaning up and returning the borrowed arrays - writer._emitted.Dispose(); - writer._intStack.Dispose(); + _emitted.Dispose(); + _intStack.Dispose(); - return new RegexCode(emitted, writer._stringTable, writer._trackCount, writer._caps, capsize, bmPrefix, fcPrefix, anchors, rtl); + return new RegexCode(emitted, _stringTable, _trackCount, _caps, capsize, bmPrefix, fcPrefix, anchors, rtl); } /// diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/ResizableValueListBuilder.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/ResizableValueListBuilder.cs deleted file mode 100644 index 8bebf5e..0000000 --- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/ResizableValueListBuilder.cs +++ /dev/null @@ -1,76 +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. - -using System.Buffers; -using System.Diagnostics; -using System.Runtime.CompilerServices; - -namespace System.Collections.Generic -{ - internal ref struct ResizableValueListBuilder - { - private Span _span; - private T[] _arrayFromPool; - private int _pos; - - public ResizableValueListBuilder(Span initialSpan) - { - _span = initialSpan; - _arrayFromPool = null; - _pos = 0; - } - - public ref T this[int index] => ref _span[index]; - - public int Length => _pos; - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Append(T item) - { - int pos = _pos; - if (pos >= _span.Length) - Grow(); - - _span[pos] = item; - _pos = pos + 1; - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public T Pop() - { - _pos--; - return _span[_pos]; - } - - public ReadOnlySpan AsReadOnlySpan() - { - return _span.Slice(0, _pos); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Dispose() - { - if (_arrayFromPool != null) - { - ArrayPool.Shared.Return(_arrayFromPool); - _arrayFromPool = null; - } - } - - private void Grow() - { - T[] array = ArrayPool.Shared.Rent(_span.Length * 2); - - bool success = _span.TryCopyTo(array); - Debug.Assert(success); - - T[] toReturn = _arrayFromPool; - _span = _arrayFromPool = array; - if (toReturn != null) - { - ArrayPool.Shared.Return(toReturn); - } - } - } -} diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/ValueListBuilder.Pop.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/ValueListBuilder.Pop.cs new file mode 100644 index 0000000..3a2e766 --- /dev/null +++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/ValueListBuilder.Pop.cs @@ -0,0 +1,23 @@ +// 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.Runtime.CompilerServices; + +namespace System.Collections.Generic +{ + /// + /// These public methods are required by RegexWriter. + /// + internal ref partial struct ValueListBuilder + { + public ref T this[int index] => ref _span[index]; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public T Pop() + { + _pos--; + return _span[_pos]; + } + } +} -- 2.7.4