Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/goto-programs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SRC = goto_convert.cpp goto_convert_function_call.cpp \
goto_trace.cpp xml_goto_trace.cpp vcd_goto_trace.cpp \
graphml_witness.cpp remove_virtual_functions.cpp \
class_hierarchy.cpp show_goto_functions.cpp get_goto_model.cpp \
slice_global_inits.cpp goto_inline_class.cpp
slice_global_inits.cpp goto_inline_class.cpp class_identifier.cpp

INCLUDES= -I ..

Expand Down
88 changes: 88 additions & 0 deletions src/goto-programs/class_identifier.cpp
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);
Copy link
Collaborator

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.


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")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit pick: you might want to introduce a temporary for components.front().get_name() as you are accessing it twice.

{
// found it
return member_expr;
}
else
{
e=member_expr;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I'm nit picking: e.swap(member_expr); should be more efficient.

}
}
}

/*******************************************************************\
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,
Copy link
Collaborator

Choose a reason for hiding this comment

The 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();
Copy link
Collaborator

Choose a reason for hiding this comment

The 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());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dereference_exprt deref(this_expr, this_expr.type().subtype()); (I guess the question on why that second argument is required still rests with @kroening).

return build_class_identifier(deref, ns);
}
21 changes: 21 additions & 0 deletions src/goto-programs/class_identifier.h
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>
Copy link
Collaborator

Choose a reason for hiding this comment

The 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

class exprt;
class namespacet;
class symbol_exprt;


exprt get_class_identifier_field(
exprt this_expr,
const symbol_typet &suggested_type,
const namespacet &ns);

#endif
65 changes: 6 additions & 59 deletions src/goto-programs/remove_virtual_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

/*******************************************************************\
Expand Down Expand Up @@ -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 &);
};

/*******************************************************************\
Expand All @@ -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:
Expand Down Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

The 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:

src/goto-programs/remove_virtual_functions.cpp:142:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/goto-programs/remove_virtual_functions.cpp:199:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
src/goto-programs/remove_virtual_functions.cpp:199:  Statement after an if should be on a new line  [readability/braces] [5]
src/goto-programs/remove_virtual_functions.cpp:200:  Statement after an if should be on a new line  [readability/braces] [5]
src/goto-programs/remove_virtual_functions.cpp:305:  Statement after an if should be on a new line  [readability/braces] [5]
src/goto-programs/remove_virtual_functions.cpp:383:  Should have a space between // and comment  [whitespace/comments] [4]

// 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)
Expand Down