C++ destructors: Ensure "this" is a fully qualified name#4471
C++ destructors: Ensure "this" is a fully qualified name#4471tautschnig merged 3 commits intodiffblue:developfrom
Conversation
446d656 to
bdbfa49
Compare
|
@kroening @peterschrammel Rebased and ready for review |
bdbfa49 to
c1df4ee
Compare
allredj
left a comment
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: c1df4ee).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/106625148
c1df4ee to
f3fd618
Compare
Typechecking does not magically expand `ID_this`, and there isn't sufficient context to use "cpp-this." Fixes: diffblue#661
f3fd618 to
90d5651
Compare
|
@kroening @peterschrammel This should now be ready for a review. |
allredj
left a comment
There was a problem hiding this comment.
This PR failed Diffblue compatibility checks (cbmc commit: 90d5651).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/107448314
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.
src/cpp/cpp_typecheck_function.cpp
Outdated
|
|
||
| if(it!=parameters.end() && | ||
| it->get_identifier()==ID_this) | ||
| if(it != parameters.end() && it->get_base_name() == ID_this) |
There was a problem hiding this comment.
Done, everywhere, in a separate commit.
The "this" parameter must carry a unique name for each class and must be stored in the symbol table. And indeed make sure that all parameters are in the symbol table, unless the method a member of a class template and is never used, in which case the method symbol must be removed. Previously we mostly did this removal, but would fail for methods with a body not involving any symbols (as we tested for the existence of some cpp_name).
Use the method provided by the parametert API rather than the low-level approach of a fixed base name.
90d5651 to
b18138c
Compare
allredj
left a comment
There was a problem hiding this comment.
This PR failed Diffblue compatibility checks (cbmc commit: b18138c).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/107471346
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.
Typechecking does not magically expand
ID_this, and there isn't sufficient context to use "cpp-this."Fixes: #661
Only the second commit is new, the first commit is #4470.