提交 80f9b1bd 编写于 作者: C Cyrus Najmabadi

Improve cases where we can remove parentheses in the simplifier.

Unify Vb and C# code for removing parens.
上级 d32c8be3
......@@ -4306,7 +4306,7 @@ void M()
{
switch (true)
{
case ((bool)default):
case (bool)default:
break;
}
}
......@@ -4390,7 +4390,7 @@ void M()
{
switch (true)
{
case ((bool)default) when true:
case (bool)default when true:
break;
}
}
......@@ -4420,7 +4420,7 @@ void M()
{
switch (true)
{
case (bool)default when (default):
case (bool)default when default:
break;
}
}
......@@ -4486,7 +4486,7 @@ class C
{
void M()
{
if (true is ((bool)default)) ;
if (true is (bool)default) ;
}
}", parameters: new TestParameters(new CSharpParseOptions(LanguageVersion.CSharp7_1)));
}
......
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections.Generic;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Extensions;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
using System.Collections.Generic;
namespace Microsoft.CodeAnalysis.CSharp.Extensions
{
......@@ -13,7 +14,15 @@ internal static class ParenthesizedExpressionSyntaxExtensions
public static bool CanRemoveParentheses(this ParenthesizedExpressionSyntax node, SemanticModel semanticModel)
{
var expression = node.Expression;
var parentExpression = node.Parent as ExpressionSyntax;
// The 'direct' expression that contains this parenthesized node. Note: in the case
// of code like: ```x is (y)``` there is an intermediary 'no-syntax' 'ConstantPattern'
// node between the 'is-pattern' node and the parenthesized expression. So we manually
// jump past that as, for all intents and purposes, we want to consider the 'is' expression
// as the parent expression of the (y) expression.
var parentExpression = node.IsParentKind(SyntaxKind.ConstantPattern)
? node.Parent.Parent as ExpressionSyntax
: node.Parent as ExpressionSyntax;
// Simplest cases:
// ((x)) -> (x)
......@@ -29,6 +38,17 @@ public static bool CanRemoveParentheses(this ParenthesizedExpressionSyntax node,
return true;
}
if (node.IsParentKind(SyntaxKind.ArrowExpressionClause))
{
return true;
}
// checked((x)) -> checked(x)
if (node.IsParentKind(SyntaxKind.CheckedExpression) ||
node.IsParentKind(SyntaxKind.UncheckedExpression))
{
return true;
}
// ((x, y)) -> (x, y)
if (expression.IsKind(SyntaxKind.TupleExpression))
{
......@@ -178,12 +198,29 @@ public static bool CanRemoveParentheses(this ParenthesizedExpressionSyntax node,
return true;
}
// case (x): -> case x:
if (node.IsParentKind(SyntaxKind.CaseSwitchLabel))
{
return true;
}
// case (x) when y: -> case x when y:
if (node.IsParentKind(SyntaxKind.ConstantPattern) &&
node.Parent.IsParentKind(SyntaxKind.CasePatternSwitchLabel))
{
return true;
}
// case x when (y): -> case x when y:
if (node.IsParentKind(SyntaxKind.WhenClause))
{
return true;
}
// Operator precedence cases:
// - If the parent is not an expression, do not remove parentheses
// - Otherwise, parentheses may be removed if doing so does not change operator associations.
return parentExpression != null
? !RemovalChangesAssociation(node, expression, parentExpression, semanticModel)
: false;
return parentExpression != null && !RemovalChangesAssociation(node, expression, parentExpression, semanticModel);
}
private static readonly ObjectPool<Stack<SyntaxNode>> s_nodeStackPool = new ObjectPool<Stack<SyntaxNode>>(() => new Stack<SyntaxNode>());
......@@ -249,7 +286,9 @@ private static bool RemovalMayIntroduceInterpolationAmbiguity(ParenthesizedExpre
return false;
}
private static bool RemovalChangesAssociation(ParenthesizedExpressionSyntax node, ExpressionSyntax expression, ExpressionSyntax parentExpression, SemanticModel semanticModel)
private static bool RemovalChangesAssociation(
ParenthesizedExpressionSyntax node, ExpressionSyntax expression,
ExpressionSyntax parentExpression, SemanticModel semanticModel)
{
var precedence = expression.GetOperatorPrecedence();
var parentPrecedence = parentExpression.GetOperatorPrecedence();
......@@ -283,41 +322,24 @@ private static bool RemovalChangesAssociation(ParenthesizedExpressionSyntax node
if (parentExpression is BinaryExpressionSyntax parentBinaryExpression)
{
// If both the expression and its parent are binary expressions and their kinds
// are the same, check to see if they are commutative (e.g. + or *).
if (parentBinaryExpression.IsKind(SyntaxKind.AddExpression, SyntaxKind.MultiplyExpression) &&
node.Expression.Kind() == parentBinaryExpression.Kind())
// are the same, and the parenthesized expression is on hte right and the
// operation is associative, it can sometimes be safe to remove these parens.
//
// i.e. if you have "a && (b && c)" it can be converted to "a && b && c"
// as that new interpretation "(a && b) && c" operates the exact same way at
// runtime.
//
// Specifically:
// 1) the operands are still executed in the same order: a, b, then c.
// So even if they have side effects, it will not matter.
// 2) the same shortcircuiting happens.
// 3) the result will always be the same (for logical operators, there are
// additional conditions that are checked for non-logical operators).
if (IsAssociative(parentBinaryExpression.Kind()) &&
node.Expression.Kind() == parentBinaryExpression.Kind() &&
parentBinaryExpression.Right == node)
{
// At this point, we know our parenthesized expression contains a binary expression.
var binaryExpression = (BinaryExpressionSyntax)node.Expression;
// Now we'll perform a few semantic checks to determine whether removal of the parentheses
// might break semantics. Note that we'll try and be fairly conservative with these. For example,
// we'll assume that failing any of these checks results in the parentheses being declared as
// necessary -- even if they could be removed depending on whether the parenthesized expression
// appears on the left or right side of the parent binary expression.
// First, does the binary expression result in an operator overload being called?
var symbolInfo = semanticModel.GetSymbolInfo(binaryExpression);
if (symbolInfo.Symbol != null)
{
if (symbolInfo.Symbol is IMethodSymbol methodSymbol &&
methodSymbol.MethodKind == MethodKind.UserDefinedOperator)
{
return true;
}
}
// Second, check the type and converted type of the binary expression. Are they the same?
var typeInfo = semanticModel.GetTypeInfo(binaryExpression);
if (typeInfo.Type != null && typeInfo.ConvertedType != null)
{
if (!typeInfo.Type.Equals(typeInfo.ConvertedType))
{
return true;
}
}
return false;
return !node.IsSafeToChangeAssociativity(node.Expression, semanticModel);
}
// Null-coalescing is right associative; removing parens from the LHS changes the association.
......@@ -343,34 +365,86 @@ private static bool RemovalChangesAssociation(ParenthesizedExpressionSyntax node
throw ExceptionUtilities.Unreachable;
}
private static bool IsAssociative(SyntaxKind kind)
{
switch (kind)
{
case SyntaxKind.AddExpression:
case SyntaxKind.MultiplyExpression:
case SyntaxKind.BitwiseOrExpression:
case SyntaxKind.ExclusiveOrExpression:
case SyntaxKind.LogicalOrExpression:
case SyntaxKind.BitwiseAndExpression:
case SyntaxKind.LogicalAndExpression:
return true;
}
return false;
}
private static bool RemovalMayIntroduceCastAmbiguity(ParenthesizedExpressionSyntax node)
{
// Be careful not to break the special case around (x)(-y)
// as defined in section 7.7.6 of the C# language specification.
//
// cases we can't remove the parens for are:
//
// (x)(+y)
// (x)(-y)
// (x)(&y) // unsafe code
// (x)(*y) // unsafe code
//
// Note: we can remove the parens if the (x) part is unambiguously a type.
// i.e. if it something like:
//
// (int)(...)
// (x[])(...)
// (X*)(...)
// (X?)(...)
// (global::X)(...)
if (node.IsParentKind(SyntaxKind.CastExpression))
{
var castExpression = (CastExpressionSyntax)node.Parent;
if (castExpression.Type is PredefinedTypeSyntax)
if (castExpression.Type.IsKind(
SyntaxKind.PredefinedType,
SyntaxKind.ArrayType,
SyntaxKind.PointerType,
SyntaxKind.NullableType))
{
return false;
}
if (castExpression.Type is NameSyntax name && StartsWithAlias(name))
{
return false;
}
var expression = node.Expression;
if (expression.IsKind(SyntaxKind.UnaryMinusExpression))
if (expression.IsKind(
SyntaxKind.UnaryMinusExpression,
SyntaxKind.UnaryPlusExpression,
SyntaxKind.PointerIndirectionExpression,
SyntaxKind.AddressOfExpression))
{
return true;
}
}
if (expression.IsKind(SyntaxKind.NumericLiteralExpression))
{
var numericLiteral = (LiteralExpressionSyntax)expression;
if (numericLiteral.Token.ValueText.StartsWith("-", StringComparison.Ordinal))
{
return true;
}
}
return false;
}
private static bool StartsWithAlias(NameSyntax name)
{
if (name.IsKind(SyntaxKind.AliasQualifiedName))
{
return true;
}
if (name is QualifiedNameSyntax qualifiedName)
{
return StartsWithAlias(qualifiedName.Left);
}
return false;
......
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
namespace Microsoft.CodeAnalysis.Extensions
{
internal static class CommonParenthesizedExpressionSyntaxExtensions
{
public static bool IsSafeToChangeAssociativity(
this SyntaxNode parenthesizedExpression, SyntaxNode innerExpression, SemanticModel semanticModel)
{
// Now we'll perform a few semantic checks to determine whether removal
// of the parentheses might break semantics. Note that we'll try and be
// fairly conservative with these. For example, we'll assume that failing
// any of these checks results in the parentheses being declared as necessary
// -- even if they could be removed depending on whether the parenthesized
// expression appears on the left or right side of the parent binary expression.
// First, does the binary expression result in an operator overload being
// called?
var symbolInfo = semanticModel.GetSymbolInfo(innerExpression);
if (AnySymbolIsUserDefinedOperator(symbolInfo))
{
return false;
}
// Second, check the type and converted type of the binary expression.
// Are they the same?
var innerTypeInfo = semanticModel.GetTypeInfo(innerExpression);
if (innerTypeInfo.Type != null && innerTypeInfo.ConvertedType != null)
{
if (!innerTypeInfo.Type.Equals(innerTypeInfo.ConvertedType))
{
return false;
}
}
// Floating point is not safe to change associativity of. For example,
// if the user has "large * (large * small)" then this will become
// "(large * large) * small. And that could easily overflow to Inf (and
// other badness).
var parentBinary = parenthesizedExpression.Parent;
var outerTypeInfo = semanticModel.GetTypeInfo(parentBinary);
if (IsFloatingPoint(innerTypeInfo) || IsFloatingPoint(outerTypeInfo))
{
return false;
}
return true;
}
private static bool AnySymbolIsUserDefinedOperator(SymbolInfo symbolInfo)
{
if (IsUserDefinedOperator(symbolInfo.Symbol))
{
return true;
}
foreach (var symbol in symbolInfo.CandidateSymbols)
{
if (IsUserDefinedOperator(symbol))
{
return true;
}
}
return false;
}
private static bool IsUserDefinedOperator(ISymbol symbol)
=> symbol is IMethodSymbol methodSymbol &&
methodSymbol.MethodKind == MethodKind.UserDefinedOperator;
private static bool IsFloatingPoint(TypeInfo typeInfo)
=> IsFloatingPoint(typeInfo.Type) || IsFloatingPoint(typeInfo.ConvertedType);
private static bool IsFloatingPoint(ITypeSymbol type)
=> type?.SpecialType == SpecialType.System_Single || type?.SpecialType == SpecialType.System_Double;
}
}
......@@ -2,6 +2,7 @@
Imports System.Runtime.CompilerServices
Imports System.Threading
Imports Microsoft.CodeAnalysis.Extensions
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions
......@@ -369,12 +370,24 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions
Return True
End If
' If both the expression and it's parent are binary expressions and their kinds
' are the same, check to see if they are commutative (e.g. + or *).
If parentBinaryExpression.IsKind(SyntaxKind.AddExpression, SyntaxKind.MultiplyExpression) AndAlso
' If both the expression and its parent are binary expressions and their kinds
' are the same, and the parenthesized expression is on the right and the
' operation is associative, it can sometimes be safe to remove these parens.
'
' i.e. if you have "a AndAlso (b AndAlso c)" it can be converted to "a AndAlso b AndAlso c"
' as that New interpretation "(a AndAlso b) AndAlso c" operates the exact same way at
' runtime.
'
' Specifically:
' 1) the operands are still executed in the same order a, b, then c.
' So even if they have side effects, it will Not matter.
' 2) the same shortcircuiting happens.
' 3) the result will always be the same (for logical operators, there are
' additional conditions that are checked for non-logical operators).
If IsAssociative(parentBinaryExpression.Kind) AndAlso
expression.Kind = parentExpression.Kind Then
Return True
Return node.IsSafeToChangeAssociativity(node.Expression, semanticModel)
End If
End If
......@@ -515,5 +528,19 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions
Return False
End Function
Private Function IsAssociative(kind As SyntaxKind) As Boolean
Select Case kind
Case SyntaxKind.AddExpression,
SyntaxKind.MultiplyExpression,
SyntaxKind.AndExpression,
SyntaxKind.AndAlsoExpression,
SyntaxKind.OrExpression,
SyntaxKind.ExclusiveOrExpression,
SyntaxKind.OrElseExpression
Return True
End Select
Return False
End Function
End Module
End Namespace
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册