Skip to content

Bug fix using format function on code_declt containing initialization #5833

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

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 5 additions & 10 deletions src/goto-instrument/dump_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -650,19 +650,14 @@ void dump_ct::cleanup_decl(
std::list<irep_idt> &local_static,
std::list<irep_idt> &local_type_decls)
{
exprt value=nil_exprt();

if(decl.operands().size()==2)
{
value=decl.op1();
decl.operands().resize(1);
}

goto_programt tmp;
tmp.add(goto_programt::make_decl(decl.symbol()));

if(value.is_not_nil())
tmp.add(goto_programt::make_assignment(decl.symbol(), value));
if(optionalt<exprt> value = decl.initial_value())
{
decl.set_initial_value({});
tmp.add(goto_programt::make_assignment(decl.symbol(), std::move(*value)));
}

tmp.add(goto_programt::make_end_function());

Expand Down
2 changes: 1 addition & 1 deletion src/goto-instrument/goto_program2code.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ goto_programt::const_targett goto_program2codet::convert_decl(
{
if(next->is_assign())
{
d.copy_to_operands(next->get_assign().rhs());
d.set_initial_value({next->get_assign().rhs()});
}
else
{
Expand Down
10 changes: 9 additions & 1 deletion src/goto-programs/goto_program.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,21 @@ class goto_programt
const code_declt &get_decl() const
{
PRECONDITION(is_decl());
return to_code_decl(code);
const auto &decl = expr_checked_cast<code_declt>(code);
INVARIANT(
!decl.initial_value(),
"code_declt in goto program may not contain initialization.");
return decl;
Comment on lines +202 to +206
Copy link
Collaborator

Choose a reason for hiding this comment

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

While the INVARIANT in set_decl is probably the right thing, this one may better be placed in goto_programt::instructiont::validate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that this PR is now approved, I am going to merge as is and clean up this as a follow-up PR.

}

/// Set the declaration for DECL
void set_decl(code_declt c)
{
PRECONDITION(is_decl());
INVARIANT(
!c.initial_value(),
"Initialization must be separated from code_declt before adding to "
"goto_instructiont.");
code = std::move(c);
}

Expand Down
1 change: 1 addition & 0 deletions src/goto-programs/wp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ exprt wp_decl(
const exprt &post,
const namespacet &ns)
{
PRECONDITION(!code.initial_value());
// Model decl(var) as var = nondet()
const exprt &var = code.symbol();
side_effect_expr_nondett nondet(var.type(), source_locationt());
Expand Down
11 changes: 7 additions & 4 deletions src/util/format_expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,14 @@ std::ostream &format_rec(std::ostream &os, const exprt &expr)
{
return os << "dead " << format(to_code_dead(code).symbol()) << ";";
}
else if(statement == ID_decl)
else if(const auto decl = expr_try_dynamic_cast<code_declt>(code))
{
const auto &declaration_symb = to_code_decl(code).symbol();
return os << "decl " << format(declaration_symb.type()) << " "
<< format(declaration_symb) << ";";
const auto &declaration_symb = decl->symbol();
os << "decl " << format(declaration_symb.type()) << " "
<< format(declaration_symb);
if(const optionalt<exprt> initial_value = decl->initial_value())
os << " = " << format(*initial_value);
return os << ";";
}
else if(statement == ID_function_call)
{
Expand Down
32 changes: 32 additions & 0 deletions src/util/std_code.h
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,38 @@ class code_declt:public codet
return symbol().get_identifier();
}

/// Returns the initial value to which the declared variable is initialized,
/// or empty in the case where no initialisation is included.
/// \note: Initial values may be present in the front end but they must be
/// separated into a separate assignment when used in a `goto_instructiont`.
optionalt<exprt> initial_value() const
{
if(operands().size() < 2)
return {};
return {op1()};
}

/// Sets the value to which this declaration initializes the declared
/// variable. Empty optional maybe passed to remove existing initialisation.
/// \note: Initial values may be present in the front end but they must be
/// separated into a separate assignment when used in a `goto_instructiont`.
void set_initial_value(optionalt<exprt> initial_value)

Choose a reason for hiding this comment

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

If these can exist with optional initial_values then I'd much prefer we had a constructor that took an initial value as the argument rather than having a separate setter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the examples I have found so far were mutating after construction. I could add the facility to set this via the constructor as well.

{
if(!initial_value)
{
operands().resize(1);
}
else if(operands().size() < 2)
{
PRECONDITION(operands().size() == 1);
add_to_operands(std::move(*initial_value));
}
else
{
op1() = std::move(*initial_value);
}
}

static void check(
const codet &code,
const validation_modet vm = validation_modet::INVARIANT)
Expand Down
1 change: 1 addition & 0 deletions unit/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ SRC += analyses/ai/ai.cpp \
util/expr.cpp \
util/expr_iterator.cpp \
util/file_util.cpp \
util/format.cpp \
util/format_number_range.cpp \
util/get_base_name.cpp \
util/graph.cpp \
Expand Down
22 changes: 22 additions & 0 deletions unit/util/format.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*******************************************************************\

Module: Unit tests for `format` function.

Author: DiffBlue Limited.

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

#include <util/format.h>
#include <util/format_expr.h>
#include <util/std_code.h>

#include <testing-utils/use_catch.h>

TEST_CASE("Format a declaration statement.", "[core][util][format]")
{
const signedbv_typet int_type{32};
code_declt declaration{symbol_exprt{"foo", int_type}};
REQUIRE(format_to_string(declaration) == "decl signedbv[32] foo;");
declaration.set_initial_value({int_type.zero_expr()});
REQUIRE(format_to_string(declaration) == "decl signedbv[32] foo = 0;");
}