Skip to content

Conversation

@kroening
Copy link
Collaborator

@kroening kroening commented Sep 7, 2018

Depends on #2697. Look at last commit only.

Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

I believe rewrite_union never needed a namespace in the first place, and the changes proposed here should actually be safe even without all the ID_union_tag work. I'd propose to rebase the new commit onto develop, making the PR truly independent.

@kroening
Copy link
Collaborator Author

kroening commented Sep 7, 2018

Unfortunately, it does need the namespace right now, or you can't look up whether the symbol_type is a union or a struct.

@tautschnig
Copy link
Collaborator

Unfortunately, it does need the namespace right now, or you can't look up whether the symbol_type is a union or a struct.

Oh yes, sorry.

Could this one be rebased onto the latest version of of #2697 or those commits be cherry-picked in here for CI to succeed?

Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

The depended-on commit has been merged, should be ready to go once rebased.

@kroening
Copy link
Collaborator Author

kroening commented Sep 7, 2018

Rebase done

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: 7170711).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/84131501

@kroening kroening merged commit 0fede8f into develop Sep 8, 2018
@kroening kroening deleted the rewrite_union_ns branch September 8, 2018 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants