-
Notifications
You must be signed in to change notification settings - Fork 273
Factor class identifier extraction out of remove virtuals #471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
/*******************************************************************\ | ||
|
||
Module: Extract class identifier | ||
|
||
Author: Chris Smowton, [email protected] | ||
|
||
\*******************************************************************/ | ||
|
||
#include "class_identifier.h" | ||
|
||
#include <util/std_expr.h> | ||
|
||
/*******************************************************************\ | ||
|
||
Function: build_class_identifier | ||
|
||
Inputs: Struct expression | ||
|
||
Outputs: Member expression giving the clsid field of the input, | ||
or its parent, grandparent, etc. | ||
|
||
Purpose: | ||
|
||
\*******************************************************************/ | ||
|
||
static exprt build_class_identifier( | ||
const exprt &src, | ||
const namespacet &ns) | ||
{ | ||
// the class identifier is in the root class | ||
exprt e=src; | ||
|
||
while(1) | ||
{ | ||
const typet &type=ns.follow(e.type()); | ||
assert(type.id()==ID_struct); | ||
|
||
const struct_typet &struct_type=to_struct_type(type); | ||
const struct_typet::componentst &components=struct_type.components(); | ||
assert(!components.empty()); | ||
|
||
member_exprt member_expr( | ||
e, | ||
components.front().get_name(), | ||
components.front().type()); | ||
|
||
if(components.front().get_name()=="@class_identifier") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit pick: you might want to introduce a temporary for |
||
{ | ||
// found it | ||
return member_expr; | ||
} | ||
else | ||
{ | ||
e=member_expr; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I'm nit picking: |
||
} | ||
} | ||
} | ||
|
||
/*******************************************************************\ | ||
|
||
Function: get_class_identifier_field | ||
|
||
Inputs: Pointer expression of any pointer type, including void*, | ||
and a recommended access type if the pointer is void-typed. | ||
|
||
Outputs: Member expression to access a class identifier, as above. | ||
|
||
Purpose: | ||
|
||
\*******************************************************************/ | ||
|
||
exprt get_class_identifier_field( | ||
exprt this_expr, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see that you modify it below, but passing exprt by value is just uncommon and thus surprising. |
||
const symbol_typet &suggested_type, | ||
const namespacet &ns) | ||
{ | ||
// Get a pointer from which we can extract a clsid. | ||
// If it's already a pointer to an object of some sort, just use it; | ||
// if it's void* then use the suggested type. | ||
|
||
assert(this_expr.type().id()==ID_pointer && | ||
"Non-pointer this-arg in remove-virtuals?"); | ||
const auto& points_to=this_expr.type().subtype(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "&" goes with points_to |
||
if(points_to==empty_typet()) | ||
this_expr=typecast_exprt(this_expr, pointer_typet(suggested_type)); | ||
exprt deref=dereference_exprt(this_expr, this_expr.type().subtype()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return build_class_identifier(deref, ns); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/*******************************************************************\ | ||
|
||
Module: Extract class identifier | ||
|
||
Author: Chris Smowton, [email protected] | ||
|
||
\*******************************************************************/ | ||
|
||
#ifndef CPROVER_GOTO_PROGRAMS_CLASS_IDENTIFIER_H | ||
#define CPROVER_GOTO_PROGRAMS_CLASS_IDENTIFIER_H | ||
|
||
#include <util/expr.h> | ||
#include <util/std_types.h> | ||
#include <util/namespace.h> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit picking: I suppose the debate whether or not to use forward declarations between @peterschrammel and myself never concluded, so I'm still in favour of
|
||
|
||
exprt get_class_identifier_field( | ||
exprt this_expr, | ||
const symbol_typet &suggested_type, | ||
const namespacet &ns); | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ Author: Daniel Kroening, [email protected] | |
#include <util/type_eq.h> | ||
|
||
#include "class_hierarchy.h" | ||
#include "class_identifier.h" | ||
#include "remove_virtual_functions.h" | ||
|
||
/*******************************************************************\ | ||
|
@@ -61,8 +62,6 @@ class remove_virtual_functionst | |
exprt get_method( | ||
const irep_idt &class_id, | ||
const irep_idt &component_name) const; | ||
|
||
exprt build_class_identifier(const exprt &); | ||
}; | ||
|
||
/*******************************************************************\ | ||
|
@@ -88,48 +87,6 @@ remove_virtual_functionst::remove_virtual_functionst( | |
|
||
/*******************************************************************\ | ||
|
||
Function: remove_virtual_functionst::build_class_identifier | ||
|
||
Inputs: | ||
|
||
Outputs: | ||
|
||
Purpose: | ||
|
||
\*******************************************************************/ | ||
|
||
exprt remove_virtual_functionst::build_class_identifier( | ||
const exprt &src) | ||
{ | ||
// the class identifier is in the root class | ||
exprt e=src; | ||
|
||
while(1) | ||
{ | ||
const typet &type=ns.follow(e.type()); | ||
assert(type.id()==ID_struct); | ||
|
||
const struct_typet &struct_type=to_struct_type(type); | ||
const struct_typet::componentst &components=struct_type.components(); | ||
assert(!components.empty()); | ||
|
||
member_exprt member_expr( | ||
e, components.front().get_name(), components.front().type()); | ||
|
||
if(components.front().get_name()=="@class_identifier") | ||
{ | ||
// found it | ||
return member_expr; | ||
} | ||
else | ||
{ | ||
e=member_expr; | ||
} | ||
} | ||
} | ||
|
||
/*******************************************************************\ | ||
|
||
Function: remove_virtual_functionst::remove_virtual_function | ||
|
||
Inputs: | ||
|
@@ -181,22 +138,12 @@ void remove_virtual_functionst::remove_virtual_function( | |
goto_programt new_code_calls; | ||
goto_programt new_code_gotos; | ||
|
||
// Get a pointer from which we can extract a clsid. | ||
// If it's already a pointer to an object of some sort, just use it; | ||
// if it's void* then use the parent of all possible candidates, | ||
// which by the nature of get_functions happens to be the last candidate. | ||
|
||
exprt this_expr=code.arguments()[0]; | ||
assert(this_expr.type().id()==ID_pointer && | ||
"Non-pointer this-arg in remove-virtuals?"); | ||
const auto &points_to=this_expr.type().subtype(); | ||
if(points_to==empty_typet()) | ||
{ | ||
symbol_typet symbol_type(functions.back().class_id); | ||
this_expr=typecast_exprt(this_expr, pointer_typet(symbol_type)); | ||
} | ||
exprt deref=dereference_exprt(this_expr, this_expr.type().subtype()); | ||
exprt c_id2=build_class_identifier(deref); | ||
// If necessary, cast to the last candidate function to get the object's clsid. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just had a look at the linter's complaints: it seems this line is too long; could you please also add a commit to address the other 5 concerns on this file, even though they aren't your changes:
|
||
// By the structure of get_functions, this is the parent of all other classes | ||
// under consideration. | ||
symbol_typet suggested_type(functions.back().class_id); | ||
exprt c_id2=get_class_identifier_field(this_expr, suggested_type, ns); | ||
|
||
goto_programt::targett last_function; | ||
for(const auto &fun : functions) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That assertion is taken care of by
to_struct_type
below.