Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1462: Inconsistent enum flag check #1476

Merged
merged 3 commits into from
May 1, 2019

Conversation

mcpiroman
Copy link
Contributor

Changes
(enumValue & SomeEnum.SomeValue) == SomeEnum.None
to
(enumValue & SomeEnum.SomeValue) == 0

@@ -763,6 +763,21 @@ TranslatedExpression TranslateCeq(Comp inst, out bool negateOutput)
.WithRR(rr);
}

TranslatedExpression TryUniteEqualityOperandType(TranslatedExpression operand, TranslatedExpression otherOperand)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using names like left and right instead of operand and otherOperand will improve readability of the code.

// so that the const 0 value is printed as 0 integer and not as enum type, e.g. EnumType.None
if (operand.ResolveResult.IsCompileTimeConstant &&
operand.ResolveResult.Type.IsCSharpPrimitiveIntegerType() &&
(int)operand.ResolveResult.ConstantValue == 0 &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operand.ResolveResult.ConstantValue might be something else than a boxed int, so this can go wrong. This makes the unit tests fail. See https://ci.appveyor.com/project/icsharpcode/ilspy/build/job/wnhtrte5t1i7g5u9/tests

@siegfriedpammer
Copy link
Member

Thank you very much for providing the fix! I have added a few comments.

@dgrunwald can you please review this?

@siegfriedpammer
Copy link
Member

Thank you for providing this fix!

@siegfriedpammer siegfriedpammer merged commit ef42ff8 into icsharpcode:master May 1, 2019
ElektroKill added a commit to dnSpyEx/ILSpy that referenced this pull request Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants