Use remove-function-pointers code for restricted function pointers #6376
Conversation
tautschnig
requested review from
chrisr-diffblue,
kroening,
martin-cs and
peterschrammel
as code owners
tautschnig
force-pushed
the
restrictions
branch
from
e4bf3fb to
0cdc654
Compare
| { | ||
| std::stringstream comment; | ||
|
|
||
| comment << "dereferenced function pointer at must be "; |
There was a problem hiding this comment.
Done, thanks! (Well, I actually removed the location.)
| } | ||
| else if(candidates.empty()) | ||
| { | ||
| comment.str("no candidates for dereferenced function pointer"); |
There was a problem hiding this comment.
My rationale for omitting this is that the location will be provided anyway, I don't think we need to repeat it.
tautschnig
force-pushed
the
restrictions
branch
from
0cdc654 to
bec1d91
Compare
tautschnig
force-pushed
the
restrictions
branch
2 times, most recently
from
d6fa9bd to
74bc2fe
Compare
There was a problem hiding this comment.
Yes. There seem to be a number of different but overlapping changes in this PR. There is a little formatting, threading the logging infrastructure through the functions, converting the generation code, catching up on the changed output, etc. I realise that in this case it is not really easy to separate these but in future, if possible, separate commits would make this easier to review.
| @@ -131,9 +101,10 @@ remove_function_pointerst::remove_function_pointerst( | |||
| } | |||
| } | |||
|
|
|||
| bool remove_function_pointerst::arg_is_type_compatible( | |||
| static bool arg_is_type_compatible( | |||
There was a problem hiding this comment.
Is there a reason for preferring static functions over private member functions?
There was a problem hiding this comment.
As I made remove_function_pointer a global function, I had to have a way to use these functions.
| goto_programt &goto_program, | ||
| const irep_idt &function_id, | ||
| goto_programt::targett target, | ||
| const std::unordered_set<symbol_exprt, irep_hash> &functions, |
There was a problem hiding this comment.
This is the key line right? It gives a way of passing a map to function pointer removal and this splitting the process into "generate function maps" and "convert code using the maps"?
Codecov Report
@@ Coverage Diff @@
## develop #6376 +/- ##
========================================
Coverage 76.04% 76.04%
========================================
Files 1546 1546
Lines 165541 165536 -5
========================================
+ Hits 125882 125888 +6
+ Misses 39659 39648 -11
Continue to review full report at Codecov.
|
You're absolutely right that I should have tried to factor this into multiple commits. I had kept this in mind even when making the changes, but struggled to make it happen. And, upon a second look, I still struggle. So unless I'm really forced to, I'd rather keep it as is. |
We had two instances of code generating an if-else sequence of function calls for function pointers. The code used with --restrict-function-pointer, however, was not able to cope with type mismatches. Make the code from remove_function_pointers re-usable in both contexts, enabling support for additional type casts. Fixes: diffblue#6368
tautschnig
force-pushed
the
restrictions
branch
from
74bc2fe to
d825d04
Compare
We had two instances of code generating an if-else sequence of function
calls for function pointers. The code used with
--restrict-function-pointer, however, was not able to cope with type
mismatches. Make the code from remove_function_pointers re-usable in
both contexts, enabling support for additional type casts.
Fixes: #6368