Use non-deprecated constructors for code*t [blocks: #3800] #3928

tautschnig · 2019-01-24T22:57:28Z

The default constructor for codet is deprecated.

Each commit message has a non-empty body, explaining why the change was made.
n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
n/a My commit message includes data points confirming performance improvements (if claimed).
My PR is restricted to a single feature or bugfix.
n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

allredj

🚫
This PR failed Diffblue compatibility checks (cbmc commit: daca5ab).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98599638
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

the cbmc commit has disappeared in the mean time (e.g. in a force-push)
the author is not in the list of contributors (e.g. first-time contributors).

The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.

kroening · 2019-01-26T13:16:46Z

src/goto-programs/goto_program.h

-      target_number(nil_target)
+    explicit instructiont(goto_program_instruction_typet _type)
+      : code(ID_nil),
+        source_location(static_cast<const source_locationt &>(get_nil_irep())),


This change adds non-trivial cost when constructing instructiont -- I am wondering whether we should instead move forward on the consolidation of the ireps in there.

How about code(static_cast<codet &>(get_nil_irep()) as a cheaper alternative? Of course the proper consolidation is the correct long-term solution.

allredj

✔️
Passed Diffblue compatibility checks (cbmc commit: 5b54142).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98710967

We do not use this anywhere except that a regression test relied on it for no particular reason.

The default constructor for codet is deprecated.

allredj

✔️
Passed Diffblue compatibility checks (cbmc commit: 55ca1ef).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98738338

martin-cs

Yes, although I am less convinced of the net value of this vs. some of the other construction PRs.

martin-cs · 2019-01-27T21:02:58Z

src/analyses/flow_insensitive_analysis.cpp

@@ -217,7 +217,6 @@ bool flow_insensitive_analysis_baset::do_function_call(
    r->location_number=0;

    goto_programt::targett t=temp.add_instruction(END_FUNCTION);
-    t->code.set(ID_identifier, code.function());


I think this might be lost from a different commit. I'm not sure it is wrong but, thought you might want to know.

Note that this is in a separate commit ("Do not sneak an ID_identifier entry into END_FUNCTION"). I ended up fixing this as the regression test touched in the very same commit had surprisingly started to fail. The root cause was that .is_nil() code doesn't get printed with --print-internal-representation, and the changes in this PR make an END_FUNCTION's code nil, unlike previously where it simply was uninitialised. So we ended up with a codet that has some content but still .is_nil().

martin-cs · 2019-01-27T21:04:17Z

src/ansi-c/c_typecheck_base.cpp

@@ -648,7 +648,7 @@ void c_typecheck_baset::typecheck_declaration(
    // mark as 'already typechecked'
    make_already_typechecked(declaration.type());

-    codet contract;
+    irept contract;


Is less type information really an improvement?

In this case: yes, because we really don't care about the container of the information, just the contents we put in it. And that declaration makes this explicit.

martin-cs · 2019-01-27T21:06:13Z

src/goto-programs/goto_convert_functions.cpp

@@ -167,7 +167,6 @@ void goto_convert_functionst::convert_function(
  goto_programt::targett end_function=tmp_end_function.add_instruction();
  end_function->type=END_FUNCTION;
  end_function->source_location=end_location;
-  end_function->code.set(ID_identifier, identifier);


As with previous comment, is this from removing function names?

See above, part of the same cleanup.

tautschnig added cleanup blocker labels Jan 24, 2019

tautschnig requested review from cesaro, chrisr-diffblue, cristina-david, kroening, martin-cs, NathanJPhillips, peterschrammel, pkesseli, smowton and thk123 as code owners January 24, 2019 22:57

tautschnig added the work in progress label Jan 25, 2019

tautschnig self-assigned this Jan 25, 2019

tautschnig force-pushed the deprecation-codet branch from 542dfb3 to daca5ab Compare January 25, 2019 10:53

tautschnig removed the work in progress label Jan 25, 2019

tautschnig assigned smowton, kroening, peterschrammel and chrisr-diffblue and unassigned tautschnig Jan 25, 2019

allredj reviewed Jan 25, 2019

View reviewed changes

tautschnig force-pushed the deprecation-codet branch from daca5ab to 5b54142 Compare January 26, 2019 13:15

kroening reviewed Jan 26, 2019

View reviewed changes

smowton approved these changes Jan 26, 2019

View reviewed changes

kroening approved these changes Jan 26, 2019

View reviewed changes

allredj reviewed Jan 26, 2019

View reviewed changes

tautschnig force-pushed the deprecation-codet branch from 5b54142 to 4ea2ff6 Compare January 26, 2019 20:42

tautschnig unassigned smowton and kroening Jan 26, 2019

tautschnig assigned martin-cs Jan 26, 2019

tautschnig force-pushed the deprecation-codet branch from 4ea2ff6 to 6c901b6 Compare January 27, 2019 10:23

tautschnig added 2 commits January 27, 2019 15:37

Do not sneak an ID_identifier entry into END_FUNCTION

d0b1e0a

We do not use this anywhere except that a regression test relied on it for no particular reason.

Avoid codet construction without a statement set

55ca1ef

The default constructor for codet is deprecated.

tautschnig force-pushed the deprecation-codet branch from 6c901b6 to 55ca1ef Compare January 27, 2019 15:38

allredj reviewed Jan 27, 2019

View reviewed changes

martin-cs approved these changes Jan 27, 2019

View reviewed changes

tautschnig merged commit 51d8243 into diffblue:develop Jan 28, 2019

tautschnig deleted the deprecation-codet branch January 28, 2019 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use non-deprecated constructors for code*t [blocks: #3800] #3928

Use non-deprecated constructors for code*t [blocks: #3800] #3928

Uh oh!

tautschnig commented Jan 24, 2019

Uh oh!

allredj left a comment

Uh oh!

kroening Jan 26, 2019

Uh oh!

tautschnig Jan 26, 2019

Uh oh!

tautschnig Jan 26, 2019

Uh oh!

allredj left a comment

Uh oh!

allredj left a comment

Uh oh!

martin-cs left a comment

Uh oh!

martin-cs Jan 27, 2019

Uh oh!

tautschnig Jan 28, 2019

Uh oh!

martin-cs Jan 27, 2019

Uh oh!

tautschnig Jan 28, 2019

Uh oh!

martin-cs Jan 27, 2019

Uh oh!

tautschnig Jan 28, 2019

Uh oh!

Uh oh!

Use non-deprecated constructors for code*t [blocks: #3800] #3928

Use non-deprecated constructors for code*t [blocks: #3800] #3928

Uh oh!

Conversation

tautschnig commented Jan 24, 2019

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

martin-cs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!