From 57e6f71bb2e19cd411b28bce179efb6e786d260d Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Wed, 29 Sep 2021 09:37:02 -0400 Subject: [PATCH 1/5] Use ReadOnlySpan properties over array fields --- .../CSharp/Portable/Parser/QuickScanner.cs | 6 ++--- .../PEWriter/InstructionOperandTypesTests.cs | 4 +-- .../PEWriter/InstructionOperandTypes.cs | 5 ++-- .../Core/Portable/Xml/XmlCharType.cs | 27 ++++++++++--------- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/QuickScanner.cs b/src/Compilers/CSharp/Portable/Parser/QuickScanner.cs index 4acec774daedb..8c409c7588fa3 100644 --- a/src/Compilers/CSharp/Portable/Parser/QuickScanner.cs +++ b/src/Compilers/CSharp/Portable/Parser/QuickScanner.cs @@ -206,14 +206,14 @@ private SyntaxToken QuickScanSyntaxToken() //localize frequently accessed fields var charWindow = TextWindow.CharacterWindow; - var charPropLength = s_charProperties.Length; + var charPropLength = CharProperties.Length; for (; i < n; i++) { char c = charWindow[i]; int uc = unchecked((int)c); - var flags = uc < charPropLength ? (CharFlags)s_charProperties[uc] : CharFlags.Complex; + var flags = uc < charPropLength ? (CharFlags)CharProperties[uc] : CharFlags.Complex; state = (QuickScanState)s_stateTransitions[(int)state, (int)flags]; // NOTE: that Bad > Done and it is the only state like that @@ -273,7 +273,7 @@ private SyntaxToken CreateQuickToken() // # is marked complex as it may start directives. // PERF: Use byte instead of CharFlags so the compiler can use array literal initialization. // The most natural type choice, Enum arrays, are not blittable due to a CLR limitation. - private static readonly byte[] s_charProperties = new[] + private static ReadOnlySpan CharProperties => new[] { // 0 .. 31 (byte)CharFlags.Complex, (byte)CharFlags.Complex, (byte)CharFlags.Complex, (byte)CharFlags.Complex, (byte)CharFlags.Complex, (byte)CharFlags.Complex, (byte)CharFlags.Complex, (byte)CharFlags.Complex, diff --git a/src/Compilers/Core/CodeAnalysisTest/PEWriter/InstructionOperandTypesTests.cs b/src/Compilers/Core/CodeAnalysisTest/PEWriter/InstructionOperandTypesTests.cs index 4f2847eb1ddac..e633f71edd818 100644 --- a/src/Compilers/Core/CodeAnalysisTest/PEWriter/InstructionOperandTypesTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/PEWriter/InstructionOperandTypesTests.cs @@ -43,8 +43,8 @@ public void OperandTypes() } } - AssertEx.Equal(OneByteOperandTypes, InstructionOperandTypes.OneByte); - AssertEx.Equal(TwoByteOperandTypes, InstructionOperandTypes.TwoByte); + AssertEx.Equal(OneByteOperandTypes, InstructionOperandTypes.OneByte.ToArray()); + AssertEx.Equal(TwoByteOperandTypes, InstructionOperandTypes.TwoByte.ToArray()); } } } diff --git a/src/Compilers/Core/Portable/PEWriter/InstructionOperandTypes.cs b/src/Compilers/Core/Portable/PEWriter/InstructionOperandTypes.cs index 4b34937925f21..dee800b95225f 100644 --- a/src/Compilers/Core/Portable/PEWriter/InstructionOperandTypes.cs +++ b/src/Compilers/Core/Portable/PEWriter/InstructionOperandTypes.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; using System.Collections.Immutable; using System.Reflection.Emit; @@ -23,7 +24,7 @@ internal static OperandType ReadOperandType(ImmutableArray il, ref int pos } // internal for testing - internal static readonly byte[] OneByte = new byte[] + internal static ReadOnlySpan OneByte => new byte[] { (byte)OperandType.InlineNone, // nop (byte)OperandType.InlineNone, // break @@ -283,7 +284,7 @@ internal static OperandType ReadOperandType(ImmutableArray il, ref int pos }; // internal for testing - internal static readonly byte[] TwoByte = new byte[] + internal static ReadOnlySpan TwoByte => new byte[] { (byte)OperandType.InlineNone, // arglist (0xfe 0x00) (byte)OperandType.InlineNone, // ceq diff --git a/src/Compilers/Core/Portable/Xml/XmlCharType.cs b/src/Compilers/Core/Portable/Xml/XmlCharType.cs index 79d9ff8d02304..f11b720c74443 100644 --- a/src/Compilers/Core/Portable/Xml/XmlCharType.cs +++ b/src/Compilers/Core/Portable/Xml/XmlCharType.cs @@ -15,6 +15,7 @@ // see instructions on how to do that below. // #define XML10_FIFTH_EDITION +using System; using System.Diagnostics; namespace Microsoft.CodeAnalysis @@ -94,7 +95,7 @@ internal static class XmlCharType #if XMLCHARTYPE_USE_LITERAL_TABLE // this tables can be re-generated by the Dump function below. - private static readonly byte[] s_charPropertiesIndex = + private static readonly byte[] s_charPropertiesIndex = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x11, 0x12, 0x13, 0x14, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, @@ -111,7 +112,7 @@ internal static class XmlCharType 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x1b, }; - private static readonly byte[] s_charProperties = + private static readonly byte[] s_charProperties = new byte[] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x11, 0x11, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd1, 0xd0, 0x50, 0xd0, 0xd0, 0xd0, 0x10, 0x50, @@ -474,7 +475,7 @@ internal static class XmlCharType 0xd0, 0xd0, 0xd0, 0xd0, 0xd0, 0xd0, 0x00, 0x00, }; - private static byte charProperties(char i) + private static byte GetCharProperties(char i) { // The index entry, table, identifies the start of the appropriate 256-entry table within s_charProperties byte table = s_charPropertiesIndex[i >> innerSizeBits]; @@ -910,7 +911,7 @@ private static byte charProperties(char i) public static bool IsWhiteSpace(char ch) { - return (charProperties(ch) & fWhitespace) != 0; + return (GetCharProperties(ch) & fWhitespace) != 0; } public static bool IsExtender(char ch) @@ -920,7 +921,7 @@ public static bool IsExtender(char ch) public static bool IsNCNameSingleChar(char ch) { - return (charProperties(ch) & fNCNameSC) != 0; + return (GetCharProperties(ch) & fNCNameSC) != 0; } #if XML10_FIFTH_EDITION @@ -955,7 +956,7 @@ public static bool IsNCNameLowSurrogateChar(char lowChar) public static bool IsStartNCNameSingleChar(char ch) { - return (charProperties(ch) & fNCStartNameSC) != 0; + return (GetCharProperties(ch) & fNCStartNameSC) != 0; } #if XML10_FIFTH_EDITION @@ -982,7 +983,7 @@ public static bool IsStartNameSingleChar(char ch) public static bool IsCharData(char ch) { - return (charProperties(ch) & fCharData) != 0; + return (GetCharProperties(ch) & fCharData) != 0; } // [13] PubidChar ::= #x20 | #xD | #xA | [a-zA-Z0-9] | [-'()+,./:=?;!*#@$_%] Section 2.3 of spec @@ -998,26 +999,26 @@ public static bool IsPubidChar(char ch) // TextChar = CharData - { 0xA, 0xD, '<', '&', ']' } internal static bool IsTextChar(char ch) { - return (charProperties(ch) & fText) != 0; + return (GetCharProperties(ch) & fText) != 0; } // AttrValueChar = CharData - { 0xA, 0xD, 0x9, '<', '>', '&', '\'', '"' } internal static bool IsAttributeValueChar(char ch) { - return (charProperties(ch) & fAttrValue) != 0; + return (GetCharProperties(ch) & fAttrValue) != 0; } // XML 1.0 Fourth Edition definitions // public static bool IsLetter(char ch) { - return (charProperties(ch) & fLetter) != 0; + return (GetCharProperties(ch) & fLetter) != 0; } // This method uses the XML 4th edition name character ranges public static bool IsNCNameCharXml4e(char ch) { - return (charProperties(ch) & fNCNameXml4e) != 0; + return (GetCharProperties(ch) & fNCNameXml4e) != 0; } // This method uses the XML 4th edition name character ranges @@ -1089,7 +1090,7 @@ internal static int IsOnlyWhitespaceWithPos(string str) { for (int i = 0; i < str.Length; i++) { - if ((charProperties(str[i]) & fWhitespace) == 0) + if ((GetCharProperties(str[i]) & fWhitespace) == 0) { return i; } @@ -1104,7 +1105,7 @@ internal static int IsOnlyCharData(string str) { for (int i = 0; i < str.Length; i++) { - if ((charProperties(str[i]) & fCharData) == 0) + if ((GetCharProperties(str[i]) & fCharData) == 0) { if (i + 1 >= str.Length || !(XmlCharType.IsHighSurrogate(str[i]) && XmlCharType.IsLowSurrogate(str[i + 1]))) { From 253e968057a9cd19aeaecd980c8fb7abb51fe9df Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Wed, 29 Sep 2021 11:54:20 -0400 Subject: [PATCH 2/5] Undo stylistic change --- src/Compilers/Core/Portable/Xml/XmlCharType.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/Core/Portable/Xml/XmlCharType.cs b/src/Compilers/Core/Portable/Xml/XmlCharType.cs index f11b720c74443..da41f0253e88d 100644 --- a/src/Compilers/Core/Portable/Xml/XmlCharType.cs +++ b/src/Compilers/Core/Portable/Xml/XmlCharType.cs @@ -95,7 +95,7 @@ internal static class XmlCharType #if XMLCHARTYPE_USE_LITERAL_TABLE // this tables can be re-generated by the Dump function below. - private static readonly byte[] s_charPropertiesIndex = new byte[] + private static readonly byte[] s_charPropertiesIndex = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x11, 0x12, 0x13, 0x14, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, @@ -112,7 +112,7 @@ internal static class XmlCharType 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x1b, }; - private static readonly byte[] s_charProperties = new byte[] + private static readonly byte[] s_charProperties = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x11, 0x11, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd1, 0xd0, 0x50, 0xd0, 0xd0, 0xd0, 0x10, 0x50, From c2981c9677a7a5b53b2e94f1fb93eac743bd2d57 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Wed, 6 Oct 2021 17:45:36 -0400 Subject: [PATCH 3/5] Implement AssertEx.Equal for ReadOnlySpan --- .../PEWriter/InstructionOperandTypesTests.cs | 4 +- src/Compilers/Test/Core/Assert/AssertEx.cs | 114 ++++++++++++++++++ 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/src/Compilers/Core/CodeAnalysisTest/PEWriter/InstructionOperandTypesTests.cs b/src/Compilers/Core/CodeAnalysisTest/PEWriter/InstructionOperandTypesTests.cs index e633f71edd818..4f2847eb1ddac 100644 --- a/src/Compilers/Core/CodeAnalysisTest/PEWriter/InstructionOperandTypesTests.cs +++ b/src/Compilers/Core/CodeAnalysisTest/PEWriter/InstructionOperandTypesTests.cs @@ -43,8 +43,8 @@ public void OperandTypes() } } - AssertEx.Equal(OneByteOperandTypes, InstructionOperandTypes.OneByte.ToArray()); - AssertEx.Equal(TwoByteOperandTypes, InstructionOperandTypes.TwoByte.ToArray()); + AssertEx.Equal(OneByteOperandTypes, InstructionOperandTypes.OneByte); + AssertEx.Equal(TwoByteOperandTypes, InstructionOperandTypes.TwoByte); } } } diff --git a/src/Compilers/Test/Core/Assert/AssertEx.cs b/src/Compilers/Test/Core/Assert/AssertEx.cs index 110bbffc1b1c6..e13c0d050a9dc 100644 --- a/src/Compilers/Test/Core/Assert/AssertEx.cs +++ b/src/Compilers/Test/Core/Assert/AssertEx.cs @@ -229,6 +229,22 @@ public static void Equal( Assert.True(false, GetAssertMessage(expected, actual, comparer, message, itemInspector, itemSeparator, expectedValueSourcePath, expectedValueSourceLine)); } + public static void Equal( + ReadOnlySpan expected, + ReadOnlySpan actual, + IEqualityComparer comparer = null, + string message = null, + string itemSeparator = null, + Func itemInspector = null, + string expectedValueSourcePath = null, + int expectedValueSourceLine = 0) + { + if (SequenceEqual(expected, actual, comparer)) + return; + + Assert.True(false, GetAssertMessage(expected, actual, comparer, message, itemInspector, itemSeparator, expectedValueSourcePath, expectedValueSourceLine)); + } + /// /// Asserts that two strings are equal, and prints a diff between the two if they are not. /// @@ -326,6 +342,22 @@ private static bool SequenceEqual(IEnumerable expected, IEnumerable act return true; } + private static bool SequenceEqual(ReadOnlySpan expected, ReadOnlySpan actual, IEqualityComparer comparer = null) + { + if (expected.Length != actual.Length) + return false; + + for (int i = 0; i < expected.Length; i++) + { + if (!(comparer is not null ? comparer.Equals(expected[i], actual[i]) : AssertEqualityComparer.Equals(expected[i], actual[i]))) + { + return false; + } + } + + return true; + } + public static void SetEqual(IEnumerable expected, IEnumerable actual, IEqualityComparer comparer = null, string message = null, string itemSeparator = "\r\n", Func itemInspector = null) { var indexes = new Dictionary(comparer); @@ -663,6 +695,88 @@ public static string GetAssertMessage( return message.ToString(); } + public static string GetAssertMessage( + ReadOnlySpan expected, + ReadOnlySpan actual, + IEqualityComparer comparer = null, + string prefix = null, + Func itemInspector = null, + string itemSeparator = null, + string expectedValueSourcePath = null, + int expectedValueSourceLine = 0) + { + if (itemInspector == null) + { + if (typeof(T) == typeof(byte)) + { + itemInspector = b => $"0x{b:X2}"; + } + else + { + itemInspector = new Func(obj => (obj != null) ? obj.ToString() : ""); + } + } + + if (itemSeparator == null) + { + if (typeof(T) == typeof(byte)) + { + itemSeparator = ", "; + } + else + { + itemSeparator = "," + Environment.NewLine; + } + } + + const int maxDisplayedExpectedEntries = 10; + var expectedString = join(itemSeparator, expected[..Math.Min(expected.Length, maxDisplayedExpectedEntries)], itemInspector); + var actualString = join(itemSeparator, actual, itemInspector); + + var message = new StringBuilder(); + + if (!string.IsNullOrEmpty(prefix)) + { + message.AppendLine(prefix); + message.AppendLine(); + } + + message.AppendLine("Expected:"); + message.AppendLine(expectedString); + if (expected.Length > maxDisplayedExpectedEntries) + { + message.AppendLine("... truncated ..."); + } + + message.AppendLine("Actual:"); + message.AppendLine(actualString); + message.AppendLine("Differences:"); + message.AppendLine(DiffUtil.DiffReport(expected.ToArray(), actual.ToArray(), itemSeparator, comparer, itemInspector)); + + if (TryGenerateExpectedSourceFileAndGetDiffLink(actualString, expected.Length, expectedValueSourcePath, expectedValueSourceLine, out var link)) + { + message.AppendLine(link); + } + + return message.ToString(); + + // Local functions + + static string join(string itemSeparator, ReadOnlySpan items, Func itemInspector) + { + var result = new StringBuilder(); + var iter = items.GetEnumerator(); + + if (iter.MoveNext()) + result.Append(itemInspector(iter.Current)); + + while (iter.MoveNext()) + result.Append($"{itemSeparator}{itemInspector(iter.Current)}"); + + return result.ToString(); + } + } + internal static bool TryGenerateExpectedSourceFileAndGetDiffLink(string actualString, int expectedLineCount, string expectedValueSourcePath, int expectedValueSourceLine, out string link) { // add a link to a .cmd file that opens a diff tool: From 01fc7a8d9bd72da82aeafcd956ad684d3263887b Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 10 Jan 2022 11:46:10 -0800 Subject: [PATCH 4/5] Update XmlCharType.cs --- src/Compilers/Core/Portable/Xml/XmlCharType.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compilers/Core/Portable/Xml/XmlCharType.cs b/src/Compilers/Core/Portable/Xml/XmlCharType.cs index da41f0253e88d..abd100cb5e31f 100644 --- a/src/Compilers/Core/Portable/Xml/XmlCharType.cs +++ b/src/Compilers/Core/Portable/Xml/XmlCharType.cs @@ -15,7 +15,6 @@ // see instructions on how to do that below. // #define XML10_FIFTH_EDITION -using System; using System.Diagnostics; namespace Microsoft.CodeAnalysis From 59b9924ee7eab5fc4a60a813a5a7d9a7dc1fd7dc Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 10 Jan 2022 11:46:33 -0800 Subject: [PATCH 5/5] Update AssertEx.cs --- src/Compilers/Test/Core/Assert/AssertEx.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Compilers/Test/Core/Assert/AssertEx.cs b/src/Compilers/Test/Core/Assert/AssertEx.cs index e13c0d050a9dc..6b83f9b7fd64e 100644 --- a/src/Compilers/Test/Core/Assert/AssertEx.cs +++ b/src/Compilers/Test/Core/Assert/AssertEx.cs @@ -760,8 +760,6 @@ public static string GetAssertMessage( return message.ToString(); - // Local functions - static string join(string itemSeparator, ReadOnlySpan items, Func itemInspector) { var result = new StringBuilder();