diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/RemoveUnnecessaryCast/RemoveUnnecessaryCastTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/RemoveUnnecessaryCast/RemoveUnnecessaryCastTests.cs index 76ab5fc0ff0156eaed6070afc5463a6917b3b91f..20c8ac394b3856149e0a9da8ef990c3a40c7690e 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/RemoveUnnecessaryCast/RemoveUnnecessaryCastTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/RemoveUnnecessaryCast/RemoveUnnecessaryCastTests.cs @@ -1943,6 +1943,132 @@ static void Main() "); } + [WorkItem(2691, "https://github.com/dotnet/roslyn/issues/2691")] + [WorkItem(2987, "https://github.com/dotnet/roslyn/issues/2987")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)] + public void DontRemoveNecessaryCastBeforePointerDereference3() + { + // Conservatively disable cast simplifications for casts involving pointer conversions. + // https://github.com/dotnet/roslyn/issues/2987 tracks improving cast simplification for this scenario. + + TestMissing( + @" +class C +{ + public unsafe float ReadSingle(byte* ptr) + { + return *[|(float*)ptr|]; + } +} +"); + } + + [WorkItem(2691, "https://github.com/dotnet/roslyn/issues/2691")] + [WorkItem(2987, "https://github.com/dotnet/roslyn/issues/2987")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)] + public void DontRemoveNumericCastInUncheckedExpression() + { + // Conservatively disable cast simplifications within explicit checked/unchecked expressions. + // https://github.com/dotnet/roslyn/issues/2987 tracks improving cast simplification for this scenario. + + TestMissing( + @" +class C +{ + private unsafe readonly byte* _endPointer; + private unsafe byte* _currentPointer; + + private unsafe void CheckBounds(int byteCount) + { + if (unchecked([|(uint)byteCount)|] > (_endPointer - _currentPointer)) + { + } + } +} +"); + } + + [WorkItem(2691, "https://github.com/dotnet/roslyn/issues/2691")] + [WorkItem(2987, "https://github.com/dotnet/roslyn/issues/2987")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)] + public void DontRemoveNumericCastInUncheckedStatement() + { + // Conservatively disable cast simplifications within explicit checked/unchecked statements. + // https://github.com/dotnet/roslyn/issues/2987 tracks improving cast simplification for this scenario. + + TestMissing( + @" +class C +{ + private unsafe readonly byte* _endPointer; + private unsafe byte* _currentPointer; + + private unsafe void CheckBounds(int byteCount) + { + unchecked + { + if (([|(uint)byteCount)|] > (_endPointer - _currentPointer)) + { + } + } + } +} +"); + } + + [WorkItem(2691, "https://github.com/dotnet/roslyn/issues/2691")] + [WorkItem(2987, "https://github.com/dotnet/roslyn/issues/2987")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)] + public void DontRemoveNumericCastInCheckedExpression() + { + // Conservatively disable cast simplifications within explicit checked/unchecked expressions. + // https://github.com/dotnet/roslyn/issues/2987 tracks improving cast simplification for this scenario. + + TestMissing( + @" +class C +{ + private unsafe readonly byte* _endPointer; + private unsafe byte* _currentPointer; + + private unsafe void CheckBounds(int byteCount) + { + if (checked([|(uint)byteCount)|] > (_endPointer - _currentPointer)) + { + } + } +} +"); + } + + [WorkItem(2691, "https://github.com/dotnet/roslyn/issues/2691")] + [WorkItem(2987, "https://github.com/dotnet/roslyn/issues/2987")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)] + public void DontRemoveNumericCastInCheckedStatement() + { + // Conservatively disable cast simplifications within explicit checked/unchecked statements. + // https://github.com/dotnet/roslyn/issues/2987 tracks improving cast simplification for this scenario. + + TestMissing( + @" +class C +{ + private unsafe readonly byte* _endPointer; + private unsafe byte* _currentPointer; + + private unsafe void CheckBounds(int byteCount) + { + checked + { + if (([|(uint)byteCount)|] > (_endPointer - _currentPointer)) + { + } + } + } +} +"); + } + [WorkItem(545894)] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)] public void DontRemoveNecessaryCastInAttribute() diff --git a/src/Workspaces/CSharp/Portable/Extensions/CastExpressionSyntaxExtensions.cs b/src/Workspaces/CSharp/Portable/Extensions/CastExpressionSyntaxExtensions.cs index 8d8e4fcc1557b069964bed17ea0d853b9737de7f..b911dbf0c25ad35d93840b4a90375b00bc74345b 100644 --- a/src/Workspaces/CSharp/Portable/Extensions/CastExpressionSyntaxExtensions.cs +++ b/src/Workspaces/CSharp/Portable/Extensions/CastExpressionSyntaxExtensions.cs @@ -28,6 +28,11 @@ private static ITypeSymbol GetOuterCastType(ExpressionSyntax expression, Semanti return semanticModel.GetTypeInfo(castExpression).Type; } + if (parentNode.IsKind(SyntaxKind.PointerIndirectionExpression)) + { + return semanticModel.GetTypeInfo(expression).Type; + } + if (parentNode.IsKind(SyntaxKind.IsExpression) || parentNode.IsKind(SyntaxKind.AsExpression)) { @@ -355,6 +360,18 @@ public static bool IsUnnecessaryCast(this CastExpressionSyntax cast, SemanticMod // Explicit reference conversions can cause an exception or data loss, hence can never be removed. return false; } + else if (expressionToCastType.IsExplicit && expressionToCastType.IsNumeric && IsInExplicitCheckedOrUncheckedContext(cast)) + { + // Don't remove any explicit numeric casts in explicit checked/unchecked context. + // https://github.com/dotnet/roslyn/issues/2987 tracks improving on this conservative approach. + return false; + } + else if (expressionToCastType.IsPointer) + { + // Don't remove any non-identity pointer conversions. + // https://github.com/dotnet/roslyn/issues/2987 tracks improving on this conservative approach. + return expressionType != null && expressionType.Equals(outerType); + } if (parentIsOrAsExpression) { @@ -540,5 +557,26 @@ private static bool IsRequiredImplicitNumericConversion(ITypeSymbol sourceType, return false; } } + + private static bool IsInExplicitCheckedOrUncheckedContext(CastExpressionSyntax cast) + { + SyntaxNode currentNode = cast; + + do + { + switch(currentNode.Kind()) + { + case SyntaxKind.UncheckedExpression: + case SyntaxKind.UncheckedStatement: + case SyntaxKind.CheckedExpression: + case SyntaxKind.CheckedStatement: + return true; + } + + currentNode = currentNode.Parent; + } while (currentNode is ExpressionSyntax || currentNode is StatementSyntax); + + return false; + } } }