Make expression trees involving enum literals to look like produced b… #4574

Merged
merged 2 commits into from Aug 15, 2015

Projects

None yet

3 participants

@VSadov
Member
VSadov commented Aug 15, 2015

…y native compiler

Native compiler exposes results of constant folding in the expression trees.
In particular, conversion from an enum literal to an underlying type is fairly consistently folded by the native compiler as it can be observed in the expression trees.

Roslyn, on the other hand, was not very consistent here.
Some cases like explicitly converted literals were folded - Ex: IntTakingMethod((int)E1.a)
Other cases like implicit conversions to underlying type introduced in the processs of type promotion in binary expressions were not.

This fixes known differences in this behavior.

Fixes #4085
Fixes #3292

@VSadov VSadov Make expression trees involving enum literals to look like produced b…
…y native compiler

Native compiler exposes results of constant folding in the expression trees.
In particular, conversion from an enum literal to an underlying type is fairly consistently folded by the native compiler as it can be observed in the expression trees.

Roslyn, on the other hand, was not very consistent here.
Some cases like explicitly converted literals were folded - Ex:  IntTakingMethod((int)E1.a)
Other cases like implicit conversions to underlying type introduced in the processs of type promotion in binary expressions were not.

This fixes known differences in this behavior.

Fixes #4085
Fixes #3292
75befe7
@VSadov
Member
VSadov commented Aug 15, 2015
@gafter gafter and 1 other commented on an outdated diff Aug 15, 2015
...e/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs
@@ -402,6 +402,20 @@ private BoundExpression VisitBinaryOperator(BinaryOperatorKind opKind, MethodSym
}
}
+ private BoundExpression PromoteEnumOperand(BoundExpression operand, BoundExpression loweredOperand, TypeSymbol promotedType, bool isChecked)
+ {
+ var literal = operand as BoundLiteral;
+ if (literal != null)
+ {
+ // for compat reasons enum literals are directly promoted into underlying values
+ return Visit(literal.Update(literal.ConstantValue, promotedType));
@gafter
gafter Aug 15, 2015 Member

This is a duplicate Visit. The underlying tree has already been visited.

This may be producing a "literal" value of a nullable type. Is that a pattern that we previously used?

@VSadov
VSadov Aug 15, 2015 Member

No, it is the loweredOperand that has been visited and now contains conversions to enum and back to the underlying type. We are visiting the literal with tweaked type directly here to skip the conversions.

@gafter
gafter Aug 15, 2015 Member

Please do not visit newly constructed nodes in the lowering phase. Construct the required result, using existing helper methods if necessary.

@VSadov
VSadov Aug 15, 2015 Member

Ah, I see. Can call Constant directly here. Thanks.

@gafter
Member
gafter commented Aug 15, 2015

Other than the duplicate Visit, LGTM

@VSadov VSadov CR feedback.
No need to use Visit to create a constant expression.
8fa47a3
@VSadov VSadov merged commit ec84084 into dotnet:master Aug 15, 2015

1 check passed

default Build finished. No test results found.
Details
@VSadov VSadov deleted the VSadov:fixEnumConversions branch Feb 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment