Clean resolve inherited component #3899

thk123 · 2019-01-23T15:28:41Z

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/
[n/a] 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.
White-space or formatting changes outside the feature-related changed lines are in commits of their own.

thk123 · 2019-01-23T15:33:37Z

Was looking at this to do the refactor of java_bytecode_convert_method, but the resolve_inherited_componentt relies on the globals already being present in the symbol table so isn't trivially applicable here, instead I will perform the tidy up requested.

allredj

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

tautschnig

I really like negative diff stats :-)

smowton

Don't refer to optionalt returned by value using a reference type -- this works due to automagical lifetime extension but gives the impression that it refers to something in-place when it's actually a local.

smowton · 2019-01-23T17:14:02Z

jbmc/src/java_bytecode/java_bytecode_convert_method.cpp

-      class_hierarchy,
-      false);
-  return inherited_method.is_valid();
+  const auto &inherited_method = get_inherited_component(


Not a reference (0394j4)

Initially thought it was C++ spec reference, and was very impressed!

smowton · 2019-01-23T17:14:09Z

jbmc/src/java_bytecode/java_bytecode_convert_method.cpp

-      symbol_table,
-      class_hierarchy,
-      true);
+  const auto &inherited_method = get_inherited_component(


smowton · 2019-01-23T17:14:16Z

jbmc/src/java_bytecode/java_bytecode_language.cpp

-            class_hierarchy,
-            true);
-        if(!referred_component.is_valid())
+        const auto &referred_component = get_inherited_component(


smowton · 2019-01-23T17:15:21Z

src/goto-programs/remove_virtual_functions.cpp

-      const resolve_inherited_componentt::inherited_componentt
-        &resolved_call = resolve_function_call(child, component_name);
-      if(resolved_call.is_valid())
+      const auto &resolved_call = resolve_function_call(child, component_name);


smowton · 2019-01-23T17:15:35Z

jbmc/src/java_bytecode/java_utils.cpp

@@ -339,22 +340,22 @@ resolve_inherited_componentt::inherited_componentt get_inherited_component(
 {
  resolve_inherited_componentt component_resolver(
    symbol_table, class_hierarchy);
-  const resolve_inherited_componentt::inherited_componentt resolved_component =
+  const auto &resolved_component =


smowton · 2019-01-23T17:16:33Z

src/goto-programs/remove_virtual_functions.cpp

@@ -449,22 +447,22 @@ void remove_virtual_functionst::get_functions(
    return get_virtual_call_target(class_id, function_name, false);
    };

-  const resolve_inherited_componentt::inherited_componentt
-    &resolved_call = get_virtual_call_target(class_id, function_name, false);
+  const auto &resolved_call =


smowton · 2019-01-23T17:17:57Z

^ (0394j4) is like your emoji thing except I don't know how to type emoji :)

thk123 · 2019-01-23T17:34:58Z

@smowton For future reference, type a : and then the name of the emoji 😛

thk123 · 2019-01-23T17:41:36Z

@smowton addressed.

peterschrammel · 2019-01-23T18:48:10Z

jbmc/src/java_bytecode/ci_lazy_methods.cpp

@@ -548,11 +548,10 @@ irep_idt ci_lazy_methodst::get_virtual_method_target(
    return irep_idt();

  resolve_inherited_componentt call_resolver(symbol_table, class_hierarchy);
-  const resolve_inherited_componentt::inherited_componentt resolved_call =
-    call_resolver(classname, call_basename, false);
+  const auto &resolved_call = call_resolver(classname, call_basename, false);


🚫 Shouldn't be a reference

allredj

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

Provides more natural syntax.

allredj

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

Remove unused code

c3e4cbd

thk123 requested review from cesaro, cristina-david, kroening, NathanJPhillips, peterschrammel, pkesseli, smowton and tautschnig as code owners January 23, 2019 15:28

thk123 added the cleanup label Jan 23, 2019

allredj reviewed Jan 23, 2019

View reviewed changes

tautschnig approved these changes Jan 23, 2019

View reviewed changes

tautschnig assigned smowton and peterschrammel Jan 23, 2019

smowton reviewed Jan 23, 2019

View reviewed changes

thk123 force-pushed the clean-resolve-inherited-component branch from 5e74f0a to 9c31349 Compare January 23, 2019 17:41

smowton approved these changes Jan 23, 2019

View reviewed changes

peterschrammel reviewed Jan 23, 2019

View reviewed changes

peterschrammel assigned thk123 and unassigned peterschrammel and smowton Jan 23, 2019

allredj reviewed Jan 23, 2019

View reviewed changes

Use optional to indicate if an inherited component is not found

5bc48f8

Provides more natural syntax.

thk123 force-pushed the clean-resolve-inherited-component branch from 9c31349 to 5bc48f8 Compare January 24, 2019 09:20

allredj reviewed Jan 24, 2019

View reviewed changes

thk123 merged commit fd3d187 into diffblue:develop Jan 24, 2019

thk123 deleted the clean-resolve-inherited-component branch January 24, 2019 12:28

Clean resolve inherited component #3899

Clean resolve inherited component #3899

Uh oh!

Conversation

thk123 commented Jan 23, 2019

Uh oh!

thk123 commented Jan 23, 2019

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

tautschnig left a comment

Choose a reason for hiding this comment

Uh oh!

smowton 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!

Choose a reason for hiding this comment

Uh oh!

smowton commented Jan 23, 2019

Uh oh!

thk123 commented Jan 23, 2019

Uh oh!

thk123 commented Jan 23, 2019

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!

Uh oh!