-
Notifications
You must be signed in to change notification settings - Fork 274
smt2 id_map: Do not use nil_typet when we mean "optional" [blocks: #3800] #3948
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
smt2 id_map: Do not use nil_typet when we mean "optional" [blocks: #3800] #3948
Conversation
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.
🚫
This PR failed Diffblue compatibility checks (cbmc commit: 8033ac4).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98644406
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
Common spurious failures:
- the cbmc commit has disappeared in the mean time (e.g. in a force-push)
- the author is not in the list of contributors (e.g. first-time contributors).
The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.
I believe it's an invariant that the type is always set -- get_fresh_id could do a .find() instead, and then .emplace could be used to insert. |
I'll re-architect this one as suggested. |
8033ac4
to
c638239
Compare
@kroening Re-architected in a way that seems overall cleaner to me - but it isn't a part of the code base that I know all that well. |
|
I think what we would want here is a variant type; so a cheap substitute is to have both the |
If this is indeed a case of a variant type, then #3962 may appeal. |
Do not silently replace their type.
Use std::map's emplace API for that purpose, and while at it also refactor all inserts into id_map.
c638239
to
7792612
Compare
It seems it only almost is a case of a variant type: it's not quite the case for |
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.
✔️
Passed Diffblue compatibility checks (cbmc commit: 7792612).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98740109
Use optionalt instead. nil_typet is deprecated.