-
Notifications
You must be signed in to change notification settings - Fork 84
Cleanup: Only have one query for must-equality between expressions #156
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
Conversation
|
As this is also a query with boolean results, it brings me flashbacks from when I implemented #130 and #137, concerning all the For example, if one analysis answers Also, in some places, e.g. This is in no way a criticism of just |
I agree.
I think this the cleaner solution, as a negation of E.g. |
|
I experimented with If this is what we want to do, I can replace all instance of the |
That's true, especially if you need to figure out what result When these issues occurred to me, I think I dug a bit in the git history and found that some things were at some point renamed and flipped to avoid the
How would it mean more queries? I think the same queries would suffice but their return values would be somewhat weird, e.g. But then the natural continuation would be to also want to do the same to all the other variants in the query result type as many of them also have their own specific bottom and top. That might be too much work again. So in that sense I feel like each result variant should be a standalone lattice (and thus there be two separate variants for boolean and reverse-boolean lattice). Each query (if implemented by an analysis) should then always return something within the appropriate variant. Top of the result lattice should be reserved for "not answered" instead of "not answered or unknown". And bottom of the result lattice should be reserved for "conflicting result types" instead of "conflicting result types or conflicting values of the same type". EDIT: And then the queries themselves should probably always have "Must" or "May" in their name, to be very clear about their behavior and result variant. At least given the lack of #33. |
At least now MustBeEqual in base.ml seems to have been with understanding that both
That's pretty much the thing I now have, right? Minus the |
Maybe
The change you committed in the meanwhile matches what I originally had in mind and as I said above, I think I'd prefer it that way for now anyway. |
…tBeSingleThreaded
|
Ok, I switched them all over to During this, I noticed that |
That seems correct. When I added the negated variant to get the meet right, I initially didn't want to remove the non-negated one in case something actually depended on it working the other way for whatever reason. I made a few additional commits here: one about flipping a MayBeNot query to MustBe for better clarity and the other for adding some missing operations which I think the default case didn't cover like the old boolean lattice. |
|
I started a run of this on some larger programs, if this doesn't lead to any curious changes, I'll merge it later today. |
|
The only thing that changed is two more cases of issue #153. |
We currently have two different queries for must equality of expressions:
ExpEq(answered byvar_eqandoctagonand used inbase)MustBeEqual(answered bybase,expRelation, andoctagonand used inbaseand for arrays)(I am to blame here, I introduced the second one in my Master's thesis).
With this PR:
MustBeEqualeverywhere whereExpEqwas usedexpRelationto always answertopfor floats (see Unsoundness in SV-COMP Prerun #141)This means that we use all analyses to answer the question whether to expressions must be equal, rather than different ones in different scenarios.