Skip to content

Re-use C type conversion in C++ front-end [blocks: #4560]#4559

Merged
tautschnig merged 1 commit intodiffblue:developfrom
tautschnig:c++-attribute-conversion-1
May 15, 2019
Merged

Re-use C type conversion in C++ front-end [blocks: #4560]#4559
tautschnig merged 1 commit intodiffblue:developfrom
tautschnig:c++-attribute-conversion-1

Conversation

@tautschnig
Copy link
Collaborator

@tautschnig tautschnig commented Apr 19, 2019 •
edited by peterschrammel
Loading

This avoids duplicating code and the need to update code in two places to add new features.

Only the last commit is new, the other two are #4557 and #4558, respectively.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@tautschnig tautschnig mentioned this pull request Apr 19, 2019
Merged
4 tasks
@tautschnig tautschnig force-pushed the c++-attribute-conversion-1 branch 4 times, most recently from 9845680 to 3362dea Compare April 22, 2019 09:03
allredj
allredj reviewed Apr 22, 2019
View reviewed changes
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.

⚠️
This PR failed Diffblue compatibility checks (cbmc commit: 9845680).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/109122612
Status will be re-evaluated on next push.
Common spurious failures include: 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); compatibility was already broken by an earlier merge.

allredj
allredj reviewed Apr 22, 2019
View reviewed changes
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.

⚠️
This PR failed Diffblue compatibility checks (cbmc commit: 3362dea).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/109123056
Status will be re-evaluated on next push.
Common spurious failures include: 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); compatibility was already broken by an earlier merge.

@tautschnig tautschnig changed the title Re-use C type conversion in C++ front-end [depends-on: #4558] Re-use C type conversion in C++ front-end [depends-on: #4558, blocks: #4560] Apr 22, 2019
@tautschnig tautschnig force-pushed the c++-attribute-conversion-1 branch from 3362dea to d4c6749 Compare April 24, 2019 10:08
@tautschnig tautschnig removed their assignment Apr 24, 2019
@tautschnig tautschnig changed the title Re-use C type conversion in C++ front-end [depends-on: #4558, blocks: #4560] Re-use C type conversion in C++ front-end [blocks: #4560] Apr 24, 2019
@tautschnig tautschnig force-pushed the c++-attribute-conversion-1 branch from d4c6749 to 705b075 Compare April 30, 2019 23:43
kroening
kroening reviewed May 15, 2019
View reviewed changes
src/cpp/cpp_convert_type.cpp
extern_cnt, noreturn_cnt, wchar_t_cnt, char16_t_cnt, char32_t_cnt,
int8_cnt, int16_cnt, int32_cnt, int64_cnt, ptr32_cnt, ptr64_cnt,
float80_cnt, float128_cnt, int128_cnt;
std::size_t wchar_t_count, char16_t_count, char32_t_count;
Copy link
Collaborator

@kroening kroening May 15, 2019

Choose a reason for hiding this comment

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

It may be worth adding a comment here that these do exist in C11, but are typedefs, whereas they are keywords in C++ since in C++11. This is in essence the reason why the C++ converter differs from the C one.

Copy link
Collaborator Author

@tautschnig tautschnig May 15, 2019

Choose a reason for hiding this comment

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

Good point - I've added a comment, echoing pretty much what you said.

@tautschnig
Re-use C type conversion in C++ front-end
f13e8f9 
This avoids duplicating code and the need to update code in two places
to add new features.
@tautschnig tautschnig force-pushed the c++-attribute-conversion-1 branch from 705b075 to f13e8f9 Compare May 15, 2019 09:49
@tautschnig tautschnig assigned tautschnig and unassigned kroening May 15, 2019
@tautschnig tautschnig merged commit aff2329 into diffblue:develop May 15, 2019
@tautschnig tautschnig deleted the c++-attribute-conversion-1 branch May 15, 2019 11:55
allredj
allredj reviewed May 15, 2019
View reviewed changes
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: f13e8f9).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/111893298

tautschnig added a commit to tautschnig/cbmc that referenced this pull request May 15, 2019
@tautschnig
Merge pull request diffblue#4560 from tautschnig/c++-attribute-conver…
26406ac 
…sion

C++ front-end: Declarator to symbol conversion follows C implementation [depends-on: diffblue#4559]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kroening kroening kroening approved these changes

@peterschrammel peterschrammel peterschrammel approved these changes

+1 more reviewer

@allredj allredj allredj left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@tautschnig tautschnig

Labels

blocker bugfix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tautschnig @kroening @peterschrammel @allredj