-
Notifications
You must be signed in to change notification settings - Fork 273
Fix simplification of pointer-object comparison [blocks: #4628] #4627
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 simplification of pointer-object comparison [blocks: #4628] #4627
Conversation
68d1fdc
to
fbf7968
Compare
3c9f1b1
to
c2a516a
Compare
The unit test doesn't currently cover all the added behaviour (e.g. the line @owen-jones-diffblue suggested you change) |
1) pointer_object((T1 *)NULL) equals pointer_object((T2 *)NULL) for any types T1, T2. Previously, this would return false, unless T1==T2. 2) Do not restrict the above to NULL, but instead let the existing logic in simplify_inequality fully simplify this. 3) Add a unit test of this code, which highlighted further bugs and limitations: the unit test previously did not set the instance of the desired dynamic object, and address-of inequalities over dynamic objects can also be simplified.
c2a516a
to
359a063
Compare
Unit test extended (and locally tested that it does fail without @owen-jones-diffblue's fix). |
{ | ||
new_inequality_ops.push_back(typecast_exprt::conditional_cast( | ||
op, new_inequality_ops.front().type())); | ||
simplify_node(new_inequality_ops.back()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought for the future: in this case I think we have a very particular simplification in mind -- presumably that nulls are typed so the cast might be folded, and that (T*)(S*)x
can become (T*)x
. However bluntly using the simplifier always risks that a much more expensive transformation might be attempted. This suggests factoring out particular simplifications as utility methods, used both during simplification and as the routine way of creating a pointer cast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That certainly is something to review all the simplifier code for. I actually have some patches in a branch already, but it's not quite a systematic review yet. Will work on this in a separate PR.
types T1, T2. Previously, this would return false, unless T1==T2.
in simplify_inequality fully simplify this.
limitations: the unit test previously did not set the instance of the
desired dynamic object, and address-of inequalities over dynamic objects
can also be simplified.