C++ destructors: Ensure "this" is a fully qualified name #4471
Conversation
tautschnig
requested review from
kroening,
peterschrammel,
pkesseli and
smowton
as code owners
tautschnig
force-pushed
the
c++-temporary-and-destructor
branch
from
446d656 to
bdbfa49
Compare
tautschnig
changed the title
|
@kroening @peterschrammel Rebased and ready for review |
tautschnig
force-pushed
the
c++-temporary-and-destructor
branch
from
bdbfa49 to
c1df4ee
Compare
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
tautschnig
force-pushed
the
c++-temporary-and-destructor
branch
from
c1df4ee to
f3fd618
Compare
Typechecking does not magically expand `ID_this`, and there isn't sufficient context to use "cpp-this." Fixes: diffblue#661
tautschnig
force-pushed
the
c++-temporary-and-destructor
branch
from
f3fd618 to
90d5651
Compare
|
@kroening @peterschrammel This should now be ready for a review. |
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
| @@ -171,8 +172,7 @@ irep_idt cpp_typecheckt::function_identifier(const typet &type) | |||
| code_typet::parameterst::const_iterator it= | |||
| parameters.begin(); | |||
|
|
|||
| 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.
tautschnig
force-pushed
the
c++-temporary-and-destructor
branch
from
90d5651 to
b18138c
Compare
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.