-
Notifications
You must be signed in to change notification settings - Fork 277
Refactor gen_nondet_init to master #1027
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
Refactor gen_nondet_init to master #1027
Conversation
ddfc227
to
9af409e
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.
It seems the comments require some cleanup.
@@ -250,6 +266,151 @@ void java_object_factoryt::gen_pointer_target_init( | |||
} | |||
} | |||
|
|||
/// Initialises a primitive or object tree rooted at `expr`, of type pointer. It | |||
/// allocates child objects as necessary and nondet-initialising their members, | |||
/// or if MUST_UPDATE_IN_PLACE is set, re-initialising already-allocated |
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.
Where is "MUST_UPDATE_IN_PLACE" set?
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.
Apologies - comment moved across from test-gen-support but is no longer valid.
// <label1>: <expr> = &tmp$<temporary_counter>; | ||
// <code from recursive call to gen_nondet_init() with | ||
// tmp$<temporary_counter>> | ||
// And the next line is labelled label2 |
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.
The above comment does not coincide with the code.
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.
How so? Since I just moved it I'm not well versed in it, but my reading of what it does matches this?
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.
The comment refers to some previous version that would genuinely introduce labels; now the code just builds an if(...) {...} else {...}
code block.
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.
I've updated the comment to read:
// if(NONDET(_Bool)
// {
// <expr> = <null pointer>
// }
// else
// {
// <code from recursive call to gen_nondet_init() with
// tmp$<temporary_counter>>
// }
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.
Thanks!
} | ||
|
||
/// Initialises an object tree rooted at `expr`, allocating child objects as | ||
/// necessary and nondet-initialising their members, or if MUST_UPDATE_IN_PLACE |
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.
"MUST_UPDATE_IN_PLACE"?
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.
Apologies - comment moved across from test-gen-support but is no longer valid.
class_identifier=struct_tag; | ||
|
||
recursion_set.insert(struct_tag); | ||
assert(!recursion_set.empty()); |
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 is this a useful assertion? (It seems to check that the STL-provided code is sound.)
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 I agree - I have removed it (it doesn't even check struct_tag
was inserted).
#if 0 | ||
irep_idt _class_identifier= | ||
_is_sub?(class_identifier.empty()?struct_tag:class_identifier):""; | ||
#endif |
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.
What is this code possibly useful 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.
Don't know - looks to have been originally written by @kroening and ifdef'd out by @mgudemann, do either of you have any comments?
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.
@thk123 should be removed, is an artifact of changing API of gen_nondet_init
and a larger refactoring for a Java front-end cleanup
feee078
to
bc8db6d
Compare
Broke up the code handling initilising a pointer into a standalone function.
Split out the logic for initilising a struct into a standalone method
Removing an assertion that validates the STL lib being correct. Changed assert to invariant that all components on a struct have a name.
bc8db6d
to
e873e00
Compare
@tautschnig Done all your requests I believe. |
@@ -250,6 +266,144 @@ void java_object_factoryt::gen_pointer_target_init( | |||
} | |||
} | |||
|
|||
/// Initialises a primitive or object tree rooted at `expr`, of type pointer. It |
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.
-ize- (whole file)
I split up get_nodet_init so this PR just does this to split up the changes (and so I can get this in sooner).
This PR is already targeted to test-gen-support (#990), but to ease merging, here is a version for master.