Dump c var overlap #6257
Dump c var overlap #6257
Conversation
This adds variable name collision checking for collision between function arguments and local function variables. Fixes diffblue#6242
This fixes unit test checking when scope collision causes variable renaming.
This adds regression tests for rol/ror using symtba2gb and goto-instrument.
Adds unit test for expr2c of rol and ror operators, testing both signed and unisgned conversions.
Tidies up the style and addresses reviewer comments: - use to_shift_exprt before call to convert_rox - remove "kind" path to try and guess bit width
This adds variable name collision checking for collision between function arguments and local function variables. Fixes diffblue#6242
This fixes unit test checking when scope collision causes variable renaming.
This adds regression tests for when names collide in dump-c output when converting from json.
TGWDB
requested review from
chrisr-diffblue,
kroening and
tautschnig
as code owners
Codecov Report
@@ Coverage Diff @@
## develop #6257 +/- ##
===========================================
+ Coverage 76.16% 76.18% +0.01%
===========================================
Files 1484 1485 +1
Lines 162173 162231 +58
===========================================
+ Hits 123525 123594 +69
+ Misses 38648 38637 -11
Continue to review full report at Codecov.
|
TGWDB
force-pushed
the
dump-c-var-overlap
branch
from
ac74654 to
51d28bd
Compare
Full details reported as: diffblue#6258
Resolve an ambiguous resolution of an overloaded function in G++ 6
This adds conversion for the rol and ror operators from CBMC internals into C code. This is done by bit-twiddling the values to achieve the outcome. Fixes diffblue#6241
| // We could also conflict with a function argument, the code below | ||
| // finds the function we're in, and checks we don't conflict with | ||
| // any argument to the function | ||
| const std::string symbol_str = id2string(symbol_id); |
There was a problem hiding this comment.
Is there a reasons this symbol name/symboilt lookup code is not the same as the code in lines 168-173 above? In fact, can the checks in this block not just be folded into that existing check block?
There was a problem hiding this comment.
The check in lines 168-173 is seeing whether the symbol we're looking at now has a conflict with a function symbol name, e.g.
void f();
void main(int argc)
{
int f;
int g;
where here f is both a local variable in main and also the function f.
The new code is finding the current function we're in (e.g. inside main above) to check for collisions between local symbols and argument symbols (i.e. above f and g symbols in the main function do not collide with argc.
I can do this, but will wait and do it after #6253 since this includes infrastructure for regression tests of this form. |
TGWDB
requested review from
allredj,
martin-cs,
peterschrammel and
romainbrenguier
as code owners
@TGWDB Did this actually happen? #6253 got merged, and now the diff in this PR looks almost as big, so I'm struggling to tell. |
|
@tautschnig Yes, two tests added for detecting different variable name collisions in this commit: |
This PR adds checking for variable scope collision between function local variables and function arguments. These were previously allowed to clash as shown in PR #6242 . This PR fixes #6242