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

Issue 13001 - Add VRP support for ternary operator ?: #3698

Merged
merged 1 commit into from Jun 29, 2014

Conversation

lionello
Copy link
Contributor

{
byte b;
foreach (const i; 0..args.length?2:3)
b = i;
Copy link
Member

Choose a reason for hiding this comment

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

You can check the exact boundaries with b = i - 130; static assert(!__traits(compiles, b = i - 131)); and similar for upper bound. The CondExp tests should probably not be mixed in with the foreach tests, and there is no need to create a new file for this test.
Also, indentation doesn't match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without foreach it will use the "Match" mechanism in the compiler and figure out that both the left and the right part of ?: can be cast to byte. I need to force it to use VRP instead.

@lionello lionello changed the title Issue 13001 - Add VRE support for ternary operator ?: Issue 13001 - Add VRP support for ternary operator ?: Jun 28, 2014
@9rnsr
Copy link
Contributor

9rnsr commented Jun 28, 2014

Looks good. After rebasing, I'll merge this.

@9rnsr
Copy link
Contributor

9rnsr commented Jun 29, 2014

Auto-merge toggled on

9rnsr added a commit that referenced this pull request Jun 29, 2014
Issue 13001 - Add VRP support for ternary operator ?:
@9rnsr 9rnsr merged commit 3126735 into dlang:master Jun 29, 2014
@lionello lionello deleted the bug13001 branch June 30, 2014 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants