[TG-634] Logical expression building utility#1424
[TG-634] Logical expression building utility#1424LAJW wants to merge 2 commits intodiffblue:developfrom LAJW:axt
Conversation
|
@reuk @jasigal @romainbrenguier Take a look if you have some spare time on your hands. |
src/solvers/refinement/axt.h
Outdated
There was a problem hiding this comment.
No, because -> is an unary operator. An alias for (*ptr).<method_name>
src/solvers/refinement/axt.h
Outdated
There was a problem hiding this comment.
Why do we cast to a string expression? Is this only going to be for the string solver?
There was a problem hiding this comment.
For now it's only used in the string solver, but ultimately this should represent index_of_exprt (taking an array_exprt on the left) this will be unified once the new version of the string solver is merged as strings will be represented by arrays.
src/solvers/refinement/axt.h
Outdated
There was a problem hiding this comment.
could we also add != for notequal_exprt?
src/solvers/refinement/axt.h
Outdated
There was a problem hiding this comment.
We may want to use this for index_exprt in case it's not a string_exprt
There was a problem hiding this comment.
I shall wait then until they become one and the same thing.
src/solvers/refinement/axt.h
Outdated
There was a problem hiding this comment.
Should use axt(binary_relation_exprt(expr_, ID_ge, rhs.expr_))
src/solvers/refinement/axt.h
Outdated
There was a problem hiding this comment.
Should use axt(binary_relation_exprt(expr_, ID_le, rhs.expr_))
|
We had this code; pretty much exactly. It was called oexpr and in util/oexpr.h. It was deleted by PR #347 : 2db625 as part of a clean up in moving to a new C++ version, having been marked as "old" 346495 . As I recall, I thought of it and suggested it to Daniel on the basis you could have one piece of code that was both concrete or symbolic depending on the types used (of course you can also achieve this by doing everything symbolically and then constant folding via simply_expr, anyway, I thought I was clever at the time), he implemented it because, why not, it then sat there and wasn't used because it turns out to not be as useful as you'd originally think and then was removed because it wasn't being used. I guess there are multiple conclusions one could draw from this story and I'll let you pick your own. |
Implements an exprt wrapper for constructing logical and arithmetic expressions using C++ operators.
martin-cs
left a comment
There was a problem hiding this comment.
Over all ... I'm not convinced it gains us that much. A slight reduction in characters in long lines but that is traded against obfuscating what is symbolic and what is literal.
As I said before, I used to think this was a great idea but I've not yet seen a compelling use-case.
|
|
||
| #include <util/expr.h> | ||
| #include <util/string_expr.h> | ||
|
|
There was a problem hiding this comment.
I would have thought this would be happier in util/ as nothing it does is specific to refinement. Also, maybe we should just resurrect oexpr.
| /// strings | ||
|
|
||
| #include <solvers/refinement/string_constraint_generator.h> | ||
| #include "axt.h" |
There was a problem hiding this comment.
Please include in the usual style.
| const array_string_exprt &s2_, | ||
| const exprt &start_index_, | ||
| const exprt &end_index_) | ||
| { |
|
Change too controversial, was attempted before and rejected. Closing. |
This PR implements an
exprtwrapper for constructing logical expression hierarchies using C++ operators.Additionally, this PR contains small refactor on one of the most used string functions to show that it's working as intended.
For example, it lets following code:
be expressed this way:
exprt a1 = end_index > start_index >>= axt(s1.length()) + end_index - start_index == axt(res.length());Part 2 of String Refinement overhaul PR