From 65d10214480426506b6a1056cfde3e3cf3bf2fa1 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 21 Mar 2023 11:37:30 +0100 Subject: [PATCH 01/12] Add cache for SymbolicValue --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 31cd08421d0..201a5c1a6e6 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -27,6 +27,8 @@ public sealed record SymbolicValue { private static ConcurrentDictionary cache = new(); + private readonly Lazy hashCode; + // Reuse instances to save memory. This "True" has the same semantic meaning and any other symbolic value with BoolConstraint.True constraint public static readonly SymbolicValue Empty = new(); public static readonly SymbolicValue This = Empty.WithConstraint(ObjectConstraint.NotNull); @@ -41,6 +43,11 @@ public sealed record SymbolicValue public IEnumerable AllConstraints => Constraints.Values; + public SymbolicValue() + { + hashCode = new(() => HashCode.DictionaryContentHash(Constraints), LazyThreadSafetyMode.ExecutionAndPublication); + } + public override string ToString() => SerializeConstraints(); @@ -83,7 +90,7 @@ public T Constraint() where T : SymbolicConstraint => Constraints.TryGetValue(typeof(T), out var value) ? (T)value : null; public override int GetHashCode() => - HashCode.DictionaryContentHash(Constraints); + hashCode.Value; public bool Equals(SymbolicValue other) => other is not null && other.Constraints.DictionaryEquals(Constraints); From 49b5c30ad72d2c6cf675f70aeb9c3ec10dd2a6da Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 21 Mar 2023 17:36:00 +0100 Subject: [PATCH 02/12] Add copy constructor to reset "Lazy" --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 201a5c1a6e6..2917954b27c 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -38,16 +38,22 @@ public sealed record SymbolicValue public static readonly SymbolicValue False = NotNull.WithConstraint(BoolConstraint.False); // SymbolicValue can have only one constraint instance of specific type at a time - private ImmutableDictionary Constraints { get; init; } = ImmutableDictionary.Empty; + private ImmutableDictionary Constraints { get; init; } public IEnumerable AllConstraints => Constraints.Values; - public SymbolicValue() + public SymbolicValue() : this(null) { hashCode = new(() => HashCode.DictionaryContentHash(Constraints), LazyThreadSafetyMode.ExecutionAndPublication); } + private SymbolicValue(SymbolicValue other) + { + Constraints = other?.Constraints ?? ImmutableDictionary.Empty; + hashCode = new(() => HashCode.DictionaryContentHash(Constraints), LazyThreadSafetyMode.ExecutionAndPublication); + } + public override string ToString() => SerializeConstraints(); From 59cf2fb6f8961a0d2567e2d9c66b687d2daf0fa5 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 21 Mar 2023 17:49:43 +0100 Subject: [PATCH 03/12] Remove duplicate code --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 2917954b27c..3b055e55ae6 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -43,10 +43,7 @@ public sealed record SymbolicValue public IEnumerable AllConstraints => Constraints.Values; - public SymbolicValue() : this(null) - { - hashCode = new(() => HashCode.DictionaryContentHash(Constraints), LazyThreadSafetyMode.ExecutionAndPublication); - } + public SymbolicValue() : this(null) { } private SymbolicValue(SymbolicValue other) { From 80c001e0176c6636065f9700dfc0118feaa42c75 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Mar 2023 11:31:04 +0100 Subject: [PATCH 04/12] Use nullable int instead on lazy --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 3b055e55ae6..4a331b54d9c 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -26,9 +26,6 @@ namespace SonarAnalyzer.SymbolicExecution.Roslyn public sealed record SymbolicValue { private static ConcurrentDictionary cache = new(); - - private readonly Lazy hashCode; - // Reuse instances to save memory. This "True" has the same semantic meaning and any other symbolic value with BoolConstraint.True constraint public static readonly SymbolicValue Empty = new(); public static readonly SymbolicValue This = Empty.WithConstraint(ObjectConstraint.NotNull); @@ -37,20 +34,28 @@ public sealed record SymbolicValue public static readonly SymbolicValue True = NotNull.WithConstraint(BoolConstraint.True); public static readonly SymbolicValue False = NotNull.WithConstraint(BoolConstraint.False); + private ImmutableDictionary constraints; + private int? constraintsHashCode; + // SymbolicValue can have only one constraint instance of specific type at a time - private ImmutableDictionary Constraints { get; init; } + private ImmutableDictionary Constraints + { + get => constraints; + init + { + if (constraints != value) + { + constraints = value; + constraintsHashCode = null; + } + } + } public IEnumerable AllConstraints => Constraints.Values; public SymbolicValue() : this(null) { } - private SymbolicValue(SymbolicValue other) - { - Constraints = other?.Constraints ?? ImmutableDictionary.Empty; - hashCode = new(() => HashCode.DictionaryContentHash(Constraints), LazyThreadSafetyMode.ExecutionAndPublication); - } - public override string ToString() => SerializeConstraints(); @@ -93,7 +98,7 @@ public T Constraint() where T : SymbolicConstraint => Constraints.TryGetValue(typeof(T), out var value) ? (T)value : null; public override int GetHashCode() => - hashCode.Value; + constraintsHashCode ??= HashCode.DictionaryContentHash(constraints); public bool Equals(SymbolicValue other) => other is not null && other.Constraints.DictionaryEquals(Constraints); From cbd6fae1c82c91ab025b69fd1a15800135d2ff4c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Mar 2023 12:11:18 +0100 Subject: [PATCH 05/12] Fix implementation --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 4a331b54d9c..8d1e6985016 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -26,6 +26,7 @@ namespace SonarAnalyzer.SymbolicExecution.Roslyn public sealed record SymbolicValue { private static ConcurrentDictionary cache = new(); + // Reuse instances to save memory. This "True" has the same semantic meaning and any other symbolic value with BoolConstraint.True constraint public static readonly SymbolicValue Empty = new(); public static readonly SymbolicValue This = Empty.WithConstraint(ObjectConstraint.NotNull); @@ -34,7 +35,7 @@ public sealed record SymbolicValue public static readonly SymbolicValue True = NotNull.WithConstraint(BoolConstraint.True); public static readonly SymbolicValue False = NotNull.WithConstraint(BoolConstraint.False); - private ImmutableDictionary constraints; + private readonly ImmutableDictionary constraints = ImmutableDictionary.Empty; private int? constraintsHashCode; // SymbolicValue can have only one constraint instance of specific type at a time @@ -54,8 +55,6 @@ private ImmutableDictionary Constraints public IEnumerable AllConstraints => Constraints.Values; - public SymbolicValue() : this(null) { } - public override string ToString() => SerializeConstraints(); From 83d3aae5f6ba30da3652f59a22fa6dd87e876623 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Mar 2023 13:06:09 +0100 Subject: [PATCH 06/12] GetHashCode tests for SymbolicValue --- .../Roslyn/SymbolicValueTest.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs index e644642a1ae..7ad1fca6087 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs @@ -335,5 +335,44 @@ public void RemoveEntry_Miss_Returns_Instance_Type() var sut = SymbolicValue.Null.WithConstraint(TestConstraint.First); sut.WithoutConstraint().Should().BeSameAs(sut).And.HaveOnlyConstraints(ObjectConstraint.Null, TestConstraint.First); } + + [TestMethod] + public void GetHashCode_ReturnsDifferentValuesForPredefinedValues() + { + SymbolicValue.Empty.GetHashCode().Should().Be(SymbolicValue.Empty.GetHashCode()); + SymbolicValue.Empty.GetHashCode().Should() + .NotBe(SymbolicValue.Null.GetHashCode()).And + .NotBe(SymbolicValue.NotNull.GetHashCode()).And + .NotBe(SymbolicValue.True.GetHashCode()).And + .NotBe(SymbolicValue.False.GetHashCode()); + } + + [TestMethod] + public void GetHashCode_PredefinedValuesAreUnique() + { + new[] + { + SymbolicValue.Empty.GetHashCode(), + SymbolicValue.NotNull.GetHashCode(), + SymbolicValue.Null.GetHashCode(), + SymbolicValue.True.GetHashCode(), + SymbolicValue.False.GetHashCode() + }.Should().OnlyHaveUniqueItems(); + } + + [TestMethod] + public void GetHashCode_UncachedValues() + { + var baseConstraint = SymbolicValue.Empty + .WithConstraint(DummyConstraint.Dummy) + .WithConstraint(TestConstraint.First) + .WithConstraint(ObjectConstraint.Null) + .WithConstraint(BoolConstraint.True); + baseConstraint.GetHashCode().Should().Be(baseConstraint.GetHashCode()); + var similar = baseConstraint.WithoutConstraint(DummyConstraint.Dummy).WithConstraint(DummyConstraint.Dummy); + similar.Should().NotBeSameAs(baseConstraint); + similar.GetHashCode().Should().Be(baseConstraint.GetHashCode()); + similar.Equals(baseConstraint).Should().BeTrue(); + } } } From 389b423688ab553b51ce46eb519f6b8582fe73ff Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Mar 2023 13:12:41 +0100 Subject: [PATCH 07/12] Add more cases. --- .../SymbolicExecution/Roslyn/SymbolicValueTest.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs index 7ad1fca6087..3f1c5ca95c0 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs @@ -348,7 +348,7 @@ public void GetHashCode_ReturnsDifferentValuesForPredefinedValues() } [TestMethod] - public void GetHashCode_PredefinedValuesAreUnique() + public void GetHashCode_DifferentValuesAreUnique() { new[] { @@ -356,7 +356,10 @@ public void GetHashCode_PredefinedValuesAreUnique() SymbolicValue.NotNull.GetHashCode(), SymbolicValue.Null.GetHashCode(), SymbolicValue.True.GetHashCode(), - SymbolicValue.False.GetHashCode() + SymbolicValue.False.GetHashCode(), + SymbolicValue.Empty.WithConstraint(DummyConstraint.Dummy).GetHashCode(), + SymbolicValue.Empty.WithConstraint(DummyConstraint.Dummy).WithConstraint(TestConstraint.First).WithConstraint(ObjectConstraint.Null).GetHashCode(), + SymbolicValue.Empty.WithConstraint(DummyConstraint.Dummy).WithConstraint(TestConstraint.First).WithConstraint(ObjectConstraint.NotNull).GetHashCode(), }.Should().OnlyHaveUniqueItems(); } From 82134d24ef95f8b627935b83ed61d0b870657193 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Mar 2023 14:00:41 +0100 Subject: [PATCH 08/12] Supress warning --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 8d1e6985016..8607eb0136c 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -97,7 +97,9 @@ public T Constraint() where T : SymbolicConstraint => Constraints.TryGetValue(typeof(T), out var value) ? (T)value : null; public override int GetHashCode() => +#pragma warning disable S2328 // GetHashCode should not reference mutable fields constraintsHashCode ??= HashCode.DictionaryContentHash(constraints); +#pragma warning restore S2328 public bool Equals(SymbolicValue other) => other is not null && other.Constraints.DictionaryEquals(Constraints); From ee16d4806ef608fdaaab23f15905fe9d194ea171 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Mar 2023 14:28:22 +0100 Subject: [PATCH 09/12] Remove supression --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 8607eb0136c..8d1e6985016 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -97,9 +97,7 @@ public T Constraint() where T : SymbolicConstraint => Constraints.TryGetValue(typeof(T), out var value) ? (T)value : null; public override int GetHashCode() => -#pragma warning disable S2328 // GetHashCode should not reference mutable fields constraintsHashCode ??= HashCode.DictionaryContentHash(constraints); -#pragma warning restore S2328 public bool Equals(SymbolicValue other) => other is not null && other.Constraints.DictionaryEquals(Constraints); From 0a5e74f1317cb61fb0d06de36339cefbb83c4ef7 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Mar 2023 16:00:15 +0100 Subject: [PATCH 10/12] Formatting --- .../SymbolicExecution/Roslyn/SymbolicValueTest.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs index 3f1c5ca95c0..f9d98c623cd 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/SymbolicValueTest.cs @@ -340,11 +340,10 @@ public void RemoveEntry_Miss_Returns_Instance_Type() public void GetHashCode_ReturnsDifferentValuesForPredefinedValues() { SymbolicValue.Empty.GetHashCode().Should().Be(SymbolicValue.Empty.GetHashCode()); - SymbolicValue.Empty.GetHashCode().Should() - .NotBe(SymbolicValue.Null.GetHashCode()).And - .NotBe(SymbolicValue.NotNull.GetHashCode()).And - .NotBe(SymbolicValue.True.GetHashCode()).And - .NotBe(SymbolicValue.False.GetHashCode()); + SymbolicValue.Empty.GetHashCode().Should().NotBe(SymbolicValue.Null.GetHashCode()) + .And.NotBe(SymbolicValue.NotNull.GetHashCode()) + .And.NotBe(SymbolicValue.True.GetHashCode()) + .And.NotBe(SymbolicValue.False.GetHashCode()); } [TestMethod] From bd2aa1a9684e24dd85519d314c9580a04b4aa40f Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Mar 2023 16:42:59 +0100 Subject: [PATCH 11/12] Rename field --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 8d1e6985016..1bcddc3e883 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -36,7 +36,7 @@ public sealed record SymbolicValue public static readonly SymbolicValue False = NotNull.WithConstraint(BoolConstraint.False); private readonly ImmutableDictionary constraints = ImmutableDictionary.Empty; - private int? constraintsHashCode; + private int? hashCode; // SymbolicValue can have only one constraint instance of specific type at a time private ImmutableDictionary Constraints @@ -47,7 +47,7 @@ private ImmutableDictionary Constraints if (constraints != value) { constraints = value; - constraintsHashCode = null; + hashCode = null; } } } @@ -97,7 +97,7 @@ public T Constraint() where T : SymbolicConstraint => Constraints.TryGetValue(typeof(T), out var value) ? (T)value : null; public override int GetHashCode() => - constraintsHashCode ??= HashCode.DictionaryContentHash(constraints); + hashCode ??= HashCode.DictionaryContentHash(constraints); public bool Equals(SymbolicValue other) => other is not null && other.Constraints.DictionaryEquals(Constraints); From f3ef93dabecdca2402d5222a6cfce6fddcb2c4ab Mon Sep 17 00:00:00 2001 From: Martin Strecker <103252490+martin-strecker-sonarsource@users.noreply.github.com> Date: Mon, 27 Mar 2023 12:01:04 +0200 Subject: [PATCH 12/12] Remove if --- .../SymbolicExecution/Roslyn/SymbolicValue.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs index 1bcddc3e883..6747d55889b 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/SymbolicValue.cs @@ -44,11 +44,8 @@ private ImmutableDictionary Constraints get => constraints; init { - if (constraints != value) - { - constraints = value; - hashCode = null; - } + constraints = value; + hashCode = null; } }