Skip to content

Java object factory: pass function ID to factory routine #4184

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

Merged

Conversation

smowton
Copy link
Contributor

@smowton smowton commented Feb 13, 2019

Previously this was accidentally dropped when used from the goto_functionst entry point.

Previously this was accidentally dropped when used from the goto_functionst entry point.
@smowton smowton force-pushed the smowton/fix/convert-nondet-function-id branch from 1dff4c3 to 8103468 Compare February 14, 2019 09:06
Copy link
Contributor

@allredj allredj left a 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: 8103468).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/100913981

Copy link
Contributor

@thk123 thk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change. If The params.function_id needs to always be function_identifier, just drop the function_identifier parameter and set the field at the callsite? This feels like a weird fudge. Further - if this is fixing some broken behaviour, it needs a test.

@smowton
Copy link
Contributor Author

smowton commented Feb 14, 2019

Discussed with @thk123 in person.
Plan:

  1. Merge this
  2. Test-gen will remove the goto_functionst entry point that only they use and take it into their codebase
  3. We should split user-supplied object factory parameters (everything except function_id) from the function ID. These should probably be stored separately.

@smowton smowton merged commit 5e69625 into diffblue:develop Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants