-
Notifications
You must be signed in to change notification settings - Fork 47
Remove z3 string theory from String wrapper #1050
New issue
Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “No 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? No Sign in to your account
Conversation
4736b35
to
ef4919e
Compare
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.
Overall, LGTM, just some minor issues
utbot-framework/src/main/java/org/utbot/engine/overrides/Byte.java
Outdated
Show resolved
Hide resolved
utbot-framework/src/main/java/org/utbot/engine/overrides/Byte.java
Outdated
Show resolved
Hide resolved
utbot-framework/src/main/java/org/utbot/engine/overrides/Byte.java
Outdated
Show resolved
Hide resolved
utbot-framework/src/main/java/org/utbot/engine/overrides/Byte.java
Outdated
Show resolved
Hide resolved
utbot-framework/src/main/java/org/utbot/engine/overrides/Integer.java
Outdated
Show resolved
Hide resolved
utbot-framework/src/main/java/org/utbot/engine/overrides/strings/UtString.java
Outdated
Show resolved
Hide resolved
74afce3
to
fee46dc
Compare
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.
LGTM, perhaps @CaelmBleidd has some functional issues
2c3e058
to
e97aee4
Compare
int offset = 0; | ||
while (value > 0) { | ||
reversed[offset] = (char) ('0' + (value % 10)); | ||
value /= value; |
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.
Why? Shouldn't it be value /= 10
?
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.
Sure. It seems I was too distracted when I fixed the previous comment. I should have noticed it myself.
int offset = 0; | ||
while (value > 0) { | ||
reversed[offset] = (char) ('0' + (value % 10)); | ||
value /= value; |
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.
Same about a division
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.
Yes, same "copy-paste" type error.
return "0"; | ||
} | ||
|
||
assume(i <= 0x7FFFFFFF); // java.lang.Integer.MAX_VALUE |
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.
Why do we have <=
, but for Byte we have just less?
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.
For Byte
, the comparison was with "max byte + 1". Replaced it with < 127
to have the same comparison in all cases.
assume(i > 0x80000000); // java.lang.Integer.MIN_VALUE | ||
assume(i != 0); |
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.
These are redundant assumes, aren't they?
if (s == -32768) | ||
return "-32768"; | ||
if (s == 0) | ||
return "0"; | ||
|
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.
Please, add brackets or write it in one line
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.
Fixed, thank you.
* Removed `UtNativeString`, `NativeStringWrapper`, string-related `UtExpression` subclasses * Implemented `toString` methods for numeric wrappers in Java * Restored temporarily disabled `testByteToString` test Complex `GenericExamples` string tests are disabled until symbolic part of the wrapper is enabled.
+ updated string tests w.r.t. running without concrete execution
9e1952e
to
bfbc69c
Compare
Description
This PR removes the code that depends on Z3 string theory, and replaces missing components with the overridden Java implementation.
Fixes #1058
Fixes #131
Type of Change
Symbolic analysis of strings may behave differently. Extra executions may be found, or executions may be missed.
How Has This Been Tested?
Automated Testing
All existing unit tests should pass.
Manual Scenario
There should be no regressions on string operations. Operations that were not supported before are not expected to work.
Sample code that I checked (correct tests are generated both with and without concrete execution).
Checklist (remove irrelevant options):
This is the author self-check list