Skip to content

Added transitive closure to change-impact based on dependancy graph #191

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
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
257 changes: 163 additions & 94 deletions src/goto-diff/change_impact.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,16 @@ class change_impactt
public:
change_impactt(
const goto_modelt &model_old,
const goto_modelt &model_new);
const goto_modelt &model_new,
impact_modet impact_mode,
bool demo_output);

void operator()();

protected:
impact_modet impact_mode;
bool demo_output;

const goto_functionst &old_goto_functions;
const namespacet ns_old;
const goto_functionst &new_goto_functions;
Expand Down Expand Up @@ -281,6 +286,18 @@ class change_impactt
goto_program_change_impactt &old_impact,
goto_program_change_impactt &new_impact);

void propogate_dep_back(const dependence_grapht::nodet &d_node,
const dependence_grapht &dep_graph,
goto_functions_change_impactt &change_impact, bool del);
void propogate_dep_forward(const dependence_grapht::nodet &d_node,
const dependence_grapht &dep_graph,
goto_functions_change_impactt &change_impact, bool del);

void demo_output_change_impact(
const irep_idt &function,
const goto_program_change_impactt &c_i,
const goto_functionst &goto_functions,
const namespacet &ns) const;
void output_change_impact(
const irep_idt &function,
const goto_program_change_impactt &c_i,
Expand Down Expand Up @@ -310,7 +327,11 @@ Function: change_impactt::change_impactt

change_impactt::change_impactt(
const goto_modelt &model_old,
const goto_modelt &model_new):
const goto_modelt &model_new,
impact_modet _impact_mode,
bool _demo_output):
impact_mode(_impact_mode),
demo_output(_demo_output),
old_goto_functions(model_old.goto_functions),
ns_old(model_old.symbol_table),
new_goto_functions(model_new.goto_functions),
Expand Down Expand Up @@ -417,36 +438,10 @@ void change_impactt::change_impact(
{
const dependence_grapht::nodet &d_node=
old_dep_graph[old_dep_graph[o_it].get_node_id()];

for(dependence_grapht::edgest::const_iterator
it=d_node.in.begin();
it!=d_node.in.end();
++it)
{
goto_programt::const_targett src=
old_dep_graph[it->first].PC;

if(it->second.get()==dep_edget::DATA ||
it->second.get()==dep_edget::BOTH)
old_change_impact[src->function][src]|=DEL_DATA_DEP;
else
old_change_impact[src->function][src]|=DEL_CTRL_DEP;
}

for(dependence_grapht::edgest::const_iterator
it=d_node.out.begin();
it!=d_node.out.end();
++it)
{
goto_programt::const_targett src=
old_dep_graph[it->first].PC;

if(it->second.get()==dep_edget::DATA ||
it->second.get()==dep_edget::BOTH)
old_change_impact[src->function][src]|=DEL_DATA_DEP;
else
old_change_impact[src->function][src]|=DEL_CTRL_DEP;
}
if (impact_mode == BACKWARD || impact_mode == BOTH)
propogate_dep_back(d_node, old_dep_graph, old_change_impact, true);
if (impact_mode == FORWARD || impact_mode == BOTH)
propogate_dep_forward(d_node, old_dep_graph, old_change_impact, true);
}
old_impact[o_it]|=DELETED;
++o_it;
Expand All @@ -458,35 +453,10 @@ void change_impactt::change_impact(
const dependence_grapht::nodet &d_node=
new_dep_graph[new_dep_graph[n_it].get_node_id()];

for(dependence_grapht::edgest::const_iterator
it=d_node.in.begin();
it!=d_node.in.end();
++it)
{
goto_programt::const_targett src=
new_dep_graph[it->first].PC;

if(it->second.get()==dep_edget::DATA ||
it->second.get()==dep_edget::BOTH)
new_change_impact[src->function][src]|=NEW_DATA_DEP;
else
new_change_impact[src->function][src]|=NEW_CTRL_DEP;
}

for(dependence_grapht::edgest::const_iterator
it=d_node.out.begin();
it!=d_node.out.end();
++it)
{
goto_programt::const_targett dst=
new_dep_graph[it->first].PC;

if(it->second.get()==dep_edget::DATA ||
it->second.get()==dep_edget::BOTH)
new_change_impact[dst->function][dst]|=NEW_DATA_DEP;
else
new_change_impact[dst->function][dst]|=NEW_CTRL_DEP;
}
if (impact_mode == BACKWARD || impact_mode == BOTH)
propogate_dep_back(d_node, new_dep_graph, new_change_impact, false);
if (impact_mode == FORWARD || impact_mode == BOTH)
propogate_dep_forward(d_node, new_dep_graph, new_change_impact, false);
}
new_impact[n_it]|=NEW;
++n_it;
Expand All @@ -495,6 +465,82 @@ void change_impactt::change_impact(
}
}


/*******************************************************************\

Function: change_impactt::propogate_dep_forward

Inputs:

Outputs:

Purpose:

\*******************************************************************/

void change_impactt::propogate_dep_forward(const dependence_grapht::nodet &d_node,
const dependence_grapht &dep_graph,
goto_functions_change_impactt &change_impact, bool del) {
for (dependence_grapht::edgest::const_iterator it = d_node.out.begin();
it != d_node.out.end(); ++it) {
goto_programt::const_targett src = dep_graph[it->first].PC;

mod_flagt data_flag = del ? DEL_DATA_DEP : NEW_DATA_DEP;
mod_flagt ctrl_flag = del ? DEL_CTRL_DEP : NEW_CTRL_DEP;

if ((change_impact[src->function][src] & data_flag)
|| (change_impact[src->function][src] & ctrl_flag))
continue;
if (it->second.get() == dep_edget::DATA
|| it->second.get() == dep_edget::BOTH)
change_impact[src->function][src] |= data_flag;
else
change_impact[src->function][src] |= ctrl_flag;
propogate_dep_forward(
dep_graph[dep_graph[src].get_node_id()], dep_graph,
change_impact, del);
}

}

/*******************************************************************\

Function: change_impactt::propogate_dep_back

Inputs:

Outputs:

Purpose:

\*******************************************************************/

void change_impactt::propogate_dep_back(const dependence_grapht::nodet &d_node,
const dependence_grapht &dep_graph,
goto_functions_change_impactt &change_impact, bool del) {
for (dependence_grapht::edgest::const_iterator it = d_node.in.begin();
it != d_node.in.end(); ++it) {
goto_programt::const_targett src = dep_graph[it->first].PC;

mod_flagt data_flag = del ? DEL_DATA_DEP : NEW_DATA_DEP;
mod_flagt ctrl_flag = del ? DEL_CTRL_DEP : NEW_CTRL_DEP;

if ((change_impact[src->function][src] & data_flag)
|| (change_impact[src->function][src] & ctrl_flag)) {
continue;
}
if (it->second.get() == dep_edget::DATA
|| it->second.get() == dep_edget::BOTH)
change_impact[src->function][src] |= data_flag;
else
change_impact[src->function][src] |= ctrl_flag;

propogate_dep_back(
dep_graph[dep_graph[src].get_node_id()], dep_graph,
change_impact, del);
}
}

/*******************************************************************\

Function: change_impactt::operator()
Expand Down Expand Up @@ -544,36 +590,12 @@ void change_impactt::operator()()
nc_it!=new_change_impact.end();
++nc_it)
{
for( ;
oc_it!=old_change_impact.end() && oc_it->first<nc_it->first;
++oc_it)
output_change_impact(
oc_it->first,
oc_it->second,
old_goto_functions,
ns_old);

if(oc_it==old_change_impact.end() || nc_it->first<oc_it->first)
output_change_impact(
nc_it->first,
nc_it->second,
new_goto_functions,
ns_new);
else
{
assert(oc_it->first==nc_it->first);

output_change_impact(
nc_it->first,
oc_it->second,
old_goto_functions,
ns_old,
nc_it->second,
new_goto_functions,
ns_new);

++oc_it;
}
if (demo_output)
demo_output_change_impact(nc_it->first, nc_it->second,
new_goto_functions, ns_new);
else
output_change_impact(nc_it->first, nc_it->second,
new_goto_functions, ns_new);
}
}

Expand Down Expand Up @@ -636,6 +658,51 @@ void change_impactt::output_change_impact(

/*******************************************************************\

Function: change_impact::demo_output_change_impact

Inputs:

Outputs:

Purpose:

\*******************************************************************/

void change_impactt::demo_output_change_impact(
const irep_idt &function,
const goto_program_change_impactt &c_i,
const goto_functionst &goto_functions,
const namespacet &ns) const
{
goto_functionst::function_mapt::const_iterator f_it=
goto_functions.function_map.find(function);
assert(f_it!=goto_functions.function_map.end());
const goto_programt &goto_program=f_it->second.body;

forall_goto_program_instructions(target, goto_program)
{
goto_program_change_impactt::const_iterator c_entry=
c_i.find(target);
const unsigned mod_flags=
c_entry==c_i.end() ? SAME : c_entry->second;

// syntactic changes are preferred over data/control-dependence
// modifications
if(mod_flags==SAME);
else if(mod_flags&NEW) {
std::cout << "added: " << id2string(target->source_location.get_line()) <<std::endl;
}
else if((mod_flags&NEW_DATA_DEP) || (mod_flags&NEW_CTRL_DEP)
|| (mod_flags&DEL_DATA_DEP) || (mod_flags&DEL_CTRL_DEP)) {
std::cout << "affected: " << id2string(target->source_location.get_line()) <<std::endl;
}
else
assert(false);
}
}

/*******************************************************************\

Function: change_impact::output_change_impact

Inputs:
Expand Down Expand Up @@ -764,7 +831,7 @@ void change_impactt::output_change_impact(
assert(false);

std::cout << prefix;
goto_program.output_instruction(o_ns, function, std::cout, o_target);
std::cout << o_target->source_location.as_string() <<std::endl;
}
}

Expand All @@ -782,8 +849,10 @@ Function: change_impact

void change_impact(
const goto_modelt &model_old,
const goto_modelt &model_new)
const goto_modelt &model_new,
impact_modet impact_mode,
bool demo_output)
{
change_impactt c(model_old, model_new);
change_impactt c(model_old, model_new, impact_mode,demo_output);
c();
}
6 changes: 5 additions & 1 deletion src/goto-diff/change_impact.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ Date: April 2016

class goto_modelt;

typedef enum {FORWARD, BACKWARD, BOTH} impact_modet;

void change_impact(
const goto_modelt &model_old,
const goto_modelt &model_new);
const goto_modelt &model_new,
impact_modet impact_mode,
bool demo_output);

#endif
13 changes: 10 additions & 3 deletions src/goto-diff/goto_diff_parse_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,13 @@ int goto_diff_parse_optionst::doit()
return 0;
}

if(cmdline.isset("change-impact"))
if(cmdline.isset("change-impact") || cmdline.isset("forward-impact") || cmdline.isset("backward-impact"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Apologies in advance for nit-picking, but to keep the code base clean: lines should, where reasonably possible, be no longer than 70 characters.

{
change_impact(goto_model1, goto_model2);
impact_modet impact_mode =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check the indent.

cmdline.isset("forward-impact") ?
FORWARD :
(cmdline.isset("backward-impact") ? BACKWARD : BOTH);
change_impact(goto_model1, goto_model2, impact_mode, cmdline.isset("demo-output"));
return 0;
}

Expand Down Expand Up @@ -536,7 +540,10 @@ void goto_diff_parse_optionst::help()
" --show-functions show functions (default)\n"
" --syntactic do syntactic diff (default)\n"
" -u | --unified output unified diff\n"
" --change-impact output unified diff with dependencies\n"
" --change-impact output unified diff with forward and backward dependencies\n"
" --forward-impact output unified diff with forward dependencies\n"
" --backward-impact output unified diff with backward dependencies\n"
" --demo-output output dependencies in demo mode\n"
"\n"
"Other options:\n"
" --version show version and exit\n"
Expand Down
3 changes: 2 additions & 1 deletion src/goto-diff/goto_diff_parse_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ class optionst;
"(json-ui)" \
"(show-goto-functions)" \
"(verbosity):(version)" \
"u(unified)(change-impact)"
"u(unified)(change-impact)(forward-impact)(backward-impact)" \
"(demo-output)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure "demo-output" is a good choice: command-line options are for eternity, make sure they are almost self-explanatory.


class goto_diff_parse_optionst:
public parse_options_baset,
Expand Down