Merge pull request #69336 from CyrusNajmabadi/patternMatcherCopies

Fix issues where internal pattern matching state was being copied

Author: CyrusNajmabadi

src/EditorFeatures/Test/Utilities/PatternMatcherTests.cs

@@ -414,6 +414,19 @@ public void TryMatchSingleWordPattern_CultureAwareSingleWordPreferCaseSensitiveE
             }
         }
 
+        [Fact]
+        public void TestCachingOfPriorResult()
+        {
+            using var matcher = PatternMatcher.CreatePatternMatcher("Goo", includeMatchedSpans: true, allowFuzzyMatching: true);
+            matcher.Matches("Go");
+
+            // Ensure that the above call ended up caching the result.
+            Assert.True(((PatternMatcher.SimplePatternMatcher)matcher).GetTestAccessor().LastCacheResultIs(areSimilar: true, candidateText: "Go"));
+
+            matcher.Matches("DefNotAMatch");
+            Assert.True(((PatternMatcher.SimplePatternMatcher)matcher).GetTestAccessor().LastCacheResultIs(areSimilar: false, candidateText: "DefNotAMatch"));
+        }
+
         private static ImmutableArray<string> PartListToSubstrings(string identifier, in TemporaryArray<TextSpan> parts)
         {
             using var result = TemporaryArray<string>.Empty;

src/Workspaces/Core/Portable/FindSymbols/SearchQuery.cs

@@ -7,7 +7,7 @@
 
 namespace Microsoft.CodeAnalysis.FindSymbols
 {
-    internal readonly struct SearchQuery : IDisposable
+    internal struct SearchQuery : IDisposable
     {
         /// <summary>The name being searched for.  Is null in the case of custom predicate searching..  But 
         /// can be used for faster index based searching when it is available.</summary> 
@@ -20,7 +20,10 @@ namespace Microsoft.CodeAnalysis.FindSymbols
         ///<summary>The predicate to fall back on if faster index searching is not possible.</summary>
         private readonly Func<string, bool> _predicate;
 
-        private readonly WordSimilarityChecker? _wordSimilarityChecker;
+        /// <summary>
+        /// Not readonly as this is mutable struct.
+        /// </summary>
+        private WordSimilarityChecker _wordSimilarityChecker;
 
         private SearchQuery(string name, SearchKind kind)
         {
@@ -41,7 +44,7 @@ private SearchQuery(string name, SearchKind kind)
                     // once and it can cache all the information it needs while it does the AreSimilar
                     // check against all the possible candidates.
                     _wordSimilarityChecker = new WordSimilarityChecker(name, substringsAreSimilar: false);
-                    _predicate = _wordSimilarityChecker.Value.AreSimilar;
+                    _predicate = _wordSimilarityChecker.AreSimilar;
                     break;
                 default:
                     throw ExceptionUtilities.UnexpectedValue(kind);
@@ -54,8 +57,8 @@ private SearchQuery(Func<string, bool> predicate)
             _predicate = predicate ?? throw new ArgumentNullException(nameof(predicate));
         }
 
-        public void Dispose()
-            => _wordSimilarityChecker?.Dispose();
+        public readonly void Dispose()
+            => _wordSimilarityChecker.Dispose();
 
         public static SearchQuery Create(string name, SearchKind kind)
             => new(name, kind);
@@ -69,7 +72,7 @@ public static SearchQuery CreateFuzzy(string name)
         public static SearchQuery CreateCustom(Func<string, bool> predicate)
             => new(predicate);
 
-        public Func<string, bool> GetPredicate()
+        public readonly Func<string, bool> GetPredicate()
             => _predicate;
     }
 }

src/Workspaces/Core/Portable/PatternMatching/ContainerPatternMatcher.cs

@@ -80,7 +80,7 @@ private bool AddMatches(string container, ref TemporaryArray<PatternMatch> match
                         i--, j--)
                 {
                     var containerName = containerParts[j];
-                    if (!MatchPatternSegment(containerName, _patternSegments[i], ref tempContainerMatches.AsRef(), fuzzyMatch))
+                    if (!MatchPatternSegment(containerName, ref _patternSegments[i], ref tempContainerMatches.AsRef(), fuzzyMatch))
                     {
                         // This container didn't match the pattern piece.  So there's no match at all.
                         return false;

src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.TextChunk.cs

@@ -32,7 +32,10 @@ private struct TextChunk : IDisposable
             /// </summary>
             public TemporaryArray<TextSpan> PatternHumps;
 
-            public readonly WordSimilarityChecker? SimilarityChecker;
+            /// <summary>
+            /// Not readonly as this value caches data within it, and so it needs to be able to mutate.
+            /// </summary>
+            public WordSimilarityChecker SimilarityChecker;
 
             public readonly bool IsLowercase;
 
@@ -44,15 +47,15 @@ public TextChunk(string text, bool allowFuzzingMatching)
 
                 this.SimilarityChecker = allowFuzzingMatching
                     ? new WordSimilarityChecker(text, substringsAreSimilar: false)
-                    : null;
+                    : default;
 
                 IsLowercase = !ContainsUpperCaseLetter(text);
             }
 
             public void Dispose()
             {
                 this.PatternHumps.Dispose();
-                this.SimilarityChecker?.Dispose();
+                this.SimilarityChecker.Dispose();
             }
         }
     }

src/Workspaces/Core/Portable/PatternMatching/PatternMatcher.cs

@@ -125,22 +125,22 @@ private static bool ContainsUpperCaseLetter(string pattern)
 
         private PatternMatch? MatchPatternChunk(
             string candidate,
-            in TextChunk patternChunk,
+            ref TextChunk patternChunk,
             bool punctuationStripped,
             bool fuzzyMatch)
         {
             return fuzzyMatch
-                ? FuzzyMatchPatternChunk(candidate, patternChunk, punctuationStripped)
+                ? FuzzyMatchPatternChunk(candidate, ref patternChunk, punctuationStripped)
                 : NonFuzzyMatchPatternChunk(candidate, patternChunk, punctuationStripped);
         }
 
         private static PatternMatch? FuzzyMatchPatternChunk(
             string candidate,
-            in TextChunk patternChunk,
+            ref TextChunk patternChunk,
             bool punctuationStripped)
         {
-            Contract.ThrowIfNull(patternChunk.SimilarityChecker);
-            if (patternChunk.SimilarityChecker.Value.AreSimilar(candidate))
+            Contract.ThrowIfTrue(patternChunk.SimilarityChecker.IsDefault);
+            if (patternChunk.SimilarityChecker.AreSimilar(candidate))
             {
                 return new PatternMatch(
                     PatternMatchKind.Fuzzy, punctuationStripped, isCaseSensitive: false, matchedSpan: null);
@@ -307,7 +307,7 @@ private static bool ContainsSpaceOrAsterisk(string text)
         /// <returns>If there's only one match, then the return value is that match. Otherwise it is null.</returns>
         private bool MatchPatternSegment(
             string candidate,
-            in PatternSegment segment,
+            ref PatternSegment segment,
             ref TemporaryArray<PatternMatch> matches,
             bool fuzzyMatch)
         {
@@ -326,7 +326,7 @@ private bool MatchPatternSegment(
             if (!ContainsSpaceOrAsterisk(segment.TotalTextChunk.Text))
             {
                 var match = MatchPatternChunk(
-                    candidate, segment.TotalTextChunk, punctuationStripped: false, fuzzyMatch: fuzzyMatch);
+                    candidate, ref segment.TotalTextChunk, punctuationStripped: false, fuzzyMatch: fuzzyMatch);
                 if (match != null)
                 {
                     matches.Add(match.Value);
@@ -354,7 +354,7 @@ private bool MatchPatternSegment(
             if (subWordTextChunks.Length == 1)
             {
                 var result = MatchPatternChunk(
-                    candidate, subWordTextChunks[0], punctuationStripped: true, fuzzyMatch: fuzzyMatch);
+                    candidate, ref subWordTextChunks[0], punctuationStripped: true, fuzzyMatch: fuzzyMatch);
                 if (result == null)
                 {
                     return false;
@@ -367,11 +367,11 @@ private bool MatchPatternSegment(
             {
                 using var tempMatches = TemporaryArray<PatternMatch>.Empty;
 
-                foreach (var subWordTextChunk in subWordTextChunks)
+                for (int i = 0, n = subWordTextChunks.Length; i < n; i++)
                 {
                     // Try to match the candidate with this word
                     var result = MatchPatternChunk(
-                        candidate, subWordTextChunk, punctuationStripped: true, fuzzyMatch: fuzzyMatch);
+                        candidate, ref subWordTextChunks[i], punctuationStripped: true, fuzzyMatch: fuzzyMatch);
                     if (result == null)
                         return false;
 

src/Workspaces/Core/Portable/PatternMatching/SimplePatternMatcher.cs

@@ -4,6 +4,7 @@
 
 #nullable disable
 
+using System;
 using System.Globalization;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Collections;
@@ -12,7 +13,7 @@ namespace Microsoft.CodeAnalysis.PatternMatching
 {
     internal partial class PatternMatcher
     {
-        private sealed partial class SimplePatternMatcher : PatternMatcher
+        internal sealed partial class SimplePatternMatcher : PatternMatcher
         {
             private PatternSegment _fullPatternSegment;
 
@@ -48,8 +49,17 @@ public override bool AddMatches(string candidate, ref TemporaryArray<PatternMatc
                     return false;
                 }
 
-                return MatchPatternSegment(candidate, in _fullPatternSegment, ref matches, fuzzyMatch: false) ||
-                       MatchPatternSegment(candidate, in _fullPatternSegment, ref matches, fuzzyMatch: true);
+                return MatchPatternSegment(candidate, ref _fullPatternSegment, ref matches, fuzzyMatch: false) ||
+                       MatchPatternSegment(candidate, ref _fullPatternSegment, ref matches, fuzzyMatch: true);
+            }
+
+            public TestAccessor GetTestAccessor()
+                => new(this);
+
+            public readonly struct TestAccessor(SimplePatternMatcher simplePatternMatcher)
+            {
+                public readonly bool LastCacheResultIs(bool areSimilar, string candidateText)
+                    => simplePatternMatcher._fullPatternSegment.TotalTextChunk.SimilarityChecker.LastCacheResultIs(areSimilar, candidateText);
             }
         }
     }

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/WordSimilarityChecker.cs

@@ -32,6 +32,8 @@ private readonly struct CacheResult(string candidate, bool areSimilar, double si
         /// </summary>
         private readonly bool _substringsAreSimilar;
 
+        public readonly bool IsDefault => _source is null;
+
         public WordSimilarityChecker(string text, bool substringsAreSimilar)
         {
             _source = text ?? throw new ArgumentNullException(nameof(text));
@@ -41,7 +43,12 @@ public WordSimilarityChecker(string text, bool substringsAreSimilar)
         }
 
         public readonly void Dispose()
-            => _editDistance.Dispose();
+        {
+            if (this.IsDefault)
+                return;
+
+            _editDistance.Dispose();
+        }
 
         public static bool AreSimilar(string originalText, string candidateText)
             => AreSimilar(originalText, candidateText, substringsAreSimilar: false);
@@ -156,5 +163,8 @@ private static double Penalty(string candidateText, string originalText)
 
             return 0;
         }
+
+        public readonly bool LastCacheResultIs(bool areSimilar, string candidateText)
+            => _lastAreSimilarResult.AreSimilar == areSimilar && _lastAreSimilarResult.CandidateText == candidateText;
     }
 }