Skip to content

Comments

C++: x != 0 support in SimpleRangeAnalysis#4131

Closed
lcartey wants to merge 1 commit intogithub:mainfrom
lcartey:cpp/better-range-analysis-for-ne
Closed

C++: x != 0 support in SimpleRangeAnalysis#4131
lcartey wants to merge 1 commit intogithub:mainfrom
lcartey:cpp/better-range-analysis-for-ne

Conversation

@lcartey
Copy link
Contributor

@lcartey lcartey commented Aug 25, 2020

Currently != does not infer any bounds beyonds those implied by the type. However, if the rhs is a constant at the start or end of the type range, then we can infer slightly tighter bounds.

Currently != does not infer any bounds beyonds those implied by the
type. However, if the rhs is a constant at the start or end of the type
range, then we can infer slightly tighter bounds.
// For x != RHS, we create bounds based on the size of the type, adjusted according to the RHS bounds:
// 1. if lowerbound(RHS) == typeUpperBound(RHS.getUnspecifiedType()) == upperbound(RHS)
// then
// x < lowerbound(RHS)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm certainly buying the soundness of this, but I think it's more clear if we change this line to x < typeUpperBound(RHS.getUnspecifiedType()) to match the else case. That way, it's clear that the only change in the inferred bound is replacing a <= with a <.

And similarly for the second case below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. Also, isn't it overly detailed to call getUnspecifiedType in pseudocode? What if I just remove that?

@MathiasVP MathiasVP added the C++ label Aug 26, 2020
@jbj jbj self-assigned this Aug 31, 2020
@jbj jbj dismissed a stale review via bd2caed September 1, 2020 11:20
@jbj jbj force-pushed the cpp/better-range-analysis-for-ne branch 2 times, most recently from b896cbc to a344280 Compare September 4, 2020 07:28
@jbj
Copy link
Contributor

jbj commented Oct 30, 2020

Superseded by #4204

@jbj jbj closed this Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants