diff --git a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs index 7b7108fe5f..2607879017 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs @@ -7059,5 +7059,38 @@ public void M(bool flag) LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.CSharp8, }.RunAsync(); } + + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.PointsToAnalysis)] + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.NullAnalysis)] + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.CopyAnalysis)] + [Fact, WorkItem(6453, "https://github.com/dotnet/roslyn-analyzers/issues/6453")] + public async Task IndexedNullCompare_NoDiagnosticAsync() + { + await VerifyCSharpAnalyzerAsync(@" +using System.Collections.Generic; + +sealed class Data +{ + public object Value { get; } + public Data(object value) + { + Value = value; + } +} + +static class Test +{ + static void Filter(List list, int j) + { + for (int i = 0; i < list.Count - 1; i++) + { + if (list[i + 1].Value != null) continue; + if (list[i].Value == null) continue; // <-------- CA1508 False positive + list.RemoveAt(i); + } + } +} +"); + } } } diff --git a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_ValueContentAnalysis.cs b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_ValueContentAnalysis.cs index 62e785244b..8c6f889b3a 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_ValueContentAnalysis.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_ValueContentAnalysis.cs @@ -3152,5 +3152,37 @@ public void M(object o) "; await VerifyCS.VerifyCodeFixAsync(source, source); } + + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.ValueContentAnalysis)] + [Trait(Traits.DataflowAnalysis, Traits.Dataflow.CopyAnalysis)] + [Fact, WorkItem(6453, "https://github.com/dotnet/roslyn-analyzers/issues/6453")] + public async Task IndexedValueCompare_NoDiagnosticAsync() + { + await VerifyCSharpAnalyzerAsync(@" +using System.Collections.Generic; + +sealed class Data +{ + public int Value { get; } + public Data(int value) + { + Value = value; + } +} + +static class Test +{ + static void Filter(List list, int j) + { + for (int i = 0; i < list.Count - 1; i++) + { + if (list[i + 1].Value != 0) continue; + if (list[i].Value == 0) continue; // <-------- CA1508 False positive + list.RemoveAt(i); + } + } +} +"); + } } } diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/DefaultPointsToValueGenerator.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/DefaultPointsToValueGenerator.cs index eba643dced..6ab03de481 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/DefaultPointsToValueGenerator.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/DefaultPointsToValueGenerator.cs @@ -43,8 +43,7 @@ public PointsToAbstractValue GetOrCreateDefaultValue(AnalysisEntity analysisEnti value = PointsToAbstractValue.Create(AbstractLocation.CreateAnalysisEntityDefaultLocation(analysisEntity), mayBeNull: true); // PERF: Do not track entity and its points to value for partial analysis for entities requiring complete analysis. - if (PointsToAnalysisKind == PointsToAnalysisKind.Complete || - !analysisEntity.IsChildOrInstanceMemberNeedingCompletePointsToAnalysis()) + if (analysisEntity.ShouldBeTrackedForPointsToAnalysis(PointsToAnalysisKind)) { _trackedEntitiesBuilder.AddEntityAndPointsToValue(analysisEntity, value); _defaultPointsToValueMapBuilder.Add(analysisEntity, value); diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs index b0eed39046..7741d3cdfc 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.cs @@ -103,7 +103,7 @@ internal static bool ShouldBeTracked(AnalysisEntity analysisEntity, PointsToAnal return false; } - return pointsToAnalysisKind == PointsToAnalysisKind.Complete || !analysisEntity.IsChildOrInstanceMemberNeedingCompletePointsToAnalysis(); + return analysisEntity.ShouldBeTrackedForPointsToAnalysis(pointsToAnalysisKind); } [Conditional("DEBUG")] diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/TrackedEntitiesBuilder.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/TrackedEntitiesBuilder.cs index b5a4d574df..47e67c194a 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/TrackedEntitiesBuilder.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/TrackedEntitiesBuilder.cs @@ -45,7 +45,7 @@ public void Dispose() public void AddEntityAndPointsToValue(AnalysisEntity analysisEntity, PointsToAbstractValue value) { - Debug.Assert(PointsToAnalysisKind == PointsToAnalysisKind.Complete || !analysisEntity.IsChildOrInstanceMemberNeedingCompletePointsToAnalysis()); + Debug.Assert(analysisEntity.ShouldBeTrackedForPointsToAnalysis(PointsToAnalysisKind)); AllEntities.Add(analysisEntity); AddTrackedPointsToValue(value); diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysis.ValueContentDataFlowOperationVisitor.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysis.ValueContentDataFlowOperationVisitor.cs index 3ac7c2d212..1e92a464fd 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysis.ValueContentDataFlowOperationVisitor.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/ValueContentAnalysis/ValueContentAnalysis.ValueContentDataFlowOperationVisitor.cs @@ -34,7 +34,10 @@ protected override void ResetAbstractValue(AnalysisEntity analysisEntity) protected override void SetAbstractValue(AnalysisEntity analysisEntity, ValueContentAbstractValue value) => SetAbstractValue(CurrentAnalysisData, analysisEntity, value); - private static void SetAbstractValue(ValueContentAnalysisData analysisData, AnalysisEntity analysisEntity, ValueContentAbstractValue value) + private void SetAbstractValue(ValueContentAnalysisData analysisData, AnalysisEntity analysisEntity, ValueContentAbstractValue value) + => SetAbstractValue(analysisData, analysisEntity, value, HasCompletePointsToAnalysisResult); + + private static void SetAbstractValue(ValueContentAnalysisData analysisData, AnalysisEntity analysisEntity, ValueContentAbstractValue value, bool hasCompletePointsToAnalysisResult) { // PERF: Avoid creating an entry if the value is the default unknown value. if (value == ValueContentAbstractValue.MayBeContainsNonLiteralState && @@ -43,7 +46,10 @@ private static void SetAbstractValue(ValueContentAnalysisData analysisData, Anal return; } - analysisData.SetAbstractValue(analysisEntity, value); + if (analysisEntity.ShouldBeTrackedForAnalysis(hasCompletePointsToAnalysisResult)) + { + analysisData.SetAbstractValue(analysisEntity, value); + } } protected override bool HasAbstractValue(AnalysisEntity analysisEntity) diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs index d00a52fb71..0eb0bec4a4 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntity.cs @@ -154,17 +154,28 @@ public bool IsChildOrInstanceMember } } - internal bool IsChildOrInstanceMemberNeedingCompletePointsToAnalysis() + internal bool ShouldBeTrackedForPointsToAnalysis(PointsToAnalysisKind pointsToAnalysisKind) + => ShouldBeTrackedForAnalysis(pointsToAnalysisKind == PointsToAnalysisKind.Complete); + + internal bool ShouldBeTrackedForAnalysis(bool hasCompletePointsToAnalysisResult) { if (!IsChildOrInstanceMember) { - return false; + // We can always perform analysis for entities which are not child or instance members. + return true; + } + + // If we have complete points to analysis result, and this child or instance member has a + // known instance location, we can perform analysis for this entity. + if (hasCompletePointsToAnalysisResult && !HasUnknownInstanceLocation) + { + return true; } - // PERF: This is the core performance optimization for partial PointsToAnalysisKind. - // We avoid tracking PointsToValues for all entities that are child or instance members, + // PERF: This is the core performance optimization when we have partial points to analysis result. + // We avoid tracking analysis values for all entities that are child or instance members, // except when they are fields or members of a value type (for example, tuple elements or struct members). - return Parent == null || !Parent.Type.HasValueCopySemantics(); + return Parent != null && Parent.Type.HasValueCopySemantics(); } public bool HasConstantValue => Symbol switch diff --git a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityDataFlowOperationVisitor.cs b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityDataFlowOperationVisitor.cs index aa9c66ff99..9b940a8f51 100644 --- a/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityDataFlowOperationVisitor.cs +++ b/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityDataFlowOperationVisitor.cs @@ -217,8 +217,7 @@ protected override void SetAbstractValueForAssignment(IOperation target, IOperat { if (AnalysisEntityFactory.TryCreate(target, out var targetAnalysisEntity)) { - if (!HasCompletePointsToAnalysisResult && - targetAnalysisEntity.IsChildOrInstanceMemberNeedingCompletePointsToAnalysis()) + if (!targetAnalysisEntity.ShouldBeTrackedForAnalysis(HasCompletePointsToAnalysisResult)) { // We are not tracking points to values for fields and properties. // So, it is not possible to accurately track value changes to target entity which is a member.