[TG-2717] Generic inheritance with pointer replacement #2051

majakusber · 2018-04-12T16:47:03Z

When initiliazing a pointer and it is replaced by a different pointer (due to inheritance) we make sure that the generic parameter specialization map is updated too. For example, if we see a pointer to Interface<Integer> and there exists a class A<T> implements Interface<T> then we will be replacing the pointer with a pointer to A<Integer>. This PR makes sure that the generic type A::T is mapped to Integer.

Since CBMC does not construct replacement pointers, this change has no effect in CBMC itself (hence no unit tests or regression tests). The change is utilized in test generator: https://github.com/diffblue/test-gen/pull/1695.

tautschnig · 2018-04-13T17:22:36Z

src/util/std_types.h

+    {
+      for(const auto &b : bases())
+      {
+        if(to_symbol_type(b.type()).get(ID_identifier) == id)


Use get_identifier() instead of get(ID_identifier). Looking at the code, has_base should actually also be amended in this way. With that looking-at-code done, I'm wondering whether this duplicate iteration over bases is a good idea? Can get_base actually be implemented by just doing the loop and has_base by doing return bool(get_base(id));?

Or rather get_base(id).has_value()?

Sure, that's likely the proper way of doing it!

thk123 · 2018-04-16T09:21:57Z

src/java_bytecode/java_object_factory.cpp

@@ -766,6 +766,13 @@ void java_object_factoryt::gen_nondet_pointer_init(
  // for java types, technical debt TG-2707
  if(!equal_java_types(replacement_pointer_type, pointer_type))
  {
+    // update generic_parameter_specialization_map if needed


Consider elaborating this comment with an example. E.g.:

// if replacement_pointer_type is Implementation<T> implements I<T, Float> // then this inserts pairs: // I::T -> Implementation::T, // I::U -> java.lang.Float

Assuming my understanding is correct?

Not quite. Here is an example of what happens here:

Assume we see a pointer to I<Integer>.

convert_pointer_type finds a class Impl<T,U> implements I<T> and builds a replacement pointer Impl<Integer, U> (note that CBMC does not build these replacement pointers, only test generator).

Below we simply input the pairs for the replacement pointer to the map (nothing inheritance-specific): Impl::T -> Integer (the second parameter is unspecialized).

thk123 · 2018-04-16T09:24:24Z

src/java_bytecode/java_types.h

+  /// in the vector of generic types.
+  /// \param type The type we are looking for.
+  /// \return The index of the type in the vector of generic types.
+  optionalt<size_t> generic_type(const reference_typet &type) const


Is there a reason to return an index (not yet looked at TG PR)? It seems like this code at least would be simpler to just return a optionalt<reference_typet>

I will need the index later on in test generator so I thought a separate function for it would be useful. Would changing the name of the function make it more clear?

thomasspriggs

Just a minor readability comment, to fix if you have time.

thomasspriggs · 2018-04-16T09:40:36Z

src/java_bytecode/java_types.h

+  {
+    const size_t &index =
+      std::find(generic_types().begin(), generic_types().end(), type) -
+      generic_types().begin();


NIT: I think this would be easier to read if the result of std::find was assigned to a temporary variable and checked against generic_types().end() in the if statement below, with the subtraction only done inside the if statement.

majakusber · 2018-04-16T13:23:05Z

@thk123 Ready for re-review.

thk123 · 2018-04-16T13:27:38Z

LGTM - just waiting on TG pointer approval

majakusber · 2018-04-16T15:54:18Z

@thk123 TG pointer passed CI, I rebased and squashed this PR.

thk123

Looks like mostly transient problems, but if you could fix the clang format error then will merge once CI goes green

When initiliazing a pointer and it is replaced by a different pointer due to inheritence make sure that the generic parameter specialization map is updated too.

These will be used in test generator to convert generic pointer types.

Now simply calls a function for base retrieval

majakusber · 2018-04-17T08:30:46Z

Fixed, waiting for CI.

majakusber · 2018-04-17T09:34:12Z

@tautschnig Just noticed we need one more review for this from a code owner, would you mind taking a look at it again?

peterschrammel

LGTM

ad62682 Merge pull request diffblue#2071 from thk123/refactor/split-string-unit-tests fc8ba88 Revert to aborting precondition for function inputs 3e2ab6f Merge pull request diffblue#2080 from diffblue/java-bytecode-dependency 6ff1eec cbmc: remove dependency on java_bytecode 0bff19b Merge pull request diffblue#2049 from karkhaz/kk-factor-goto-model-processing 79e3b25 Merge pull request diffblue#2084 from tautschnig/has_subtype-test cd45b0b Test has_subtype on recursive data types 85ac315 Merge pull request diffblue#2082 from thomasspriggs/default_dstring_hash 28c2e8b Merge pull request diffblue#2065 from tautschnig/be-constructor afa6023 Merge pull request diffblue#2061 from tautschnig/simplify-extractbits 014d151 Factor out getting & processing goto-model 06b3adc Merge pull request diffblue#2077 from peterschrammel/stable-tmp-var-names 0b3170d Stabilize clinit wrapper function type parameters 3cd8bf4 Temporary vars tests for goto-diff 9f0626c Prefix temporary var names with function id ca678aa More permissive regression tests regarding tmp var suffixes 47951ca Merge pull request diffblue#2079 from romainbrenguier/bugfix/has-subtype-recursion dd73b1a Specify default hash function of `dstringt` to STL. fe8e589 Avoid infinite recursion in has_subtype 00b9bf6 Merge pull request diffblue#2051 from svorenova/generics_tg2717 cd4bfc3 Merge pull request diffblue#2078 from romainbrenguier/bool-literal-in-while-loop 67ea889 Use bool literal in while loop d229ad9 Merge pull request diffblue#2056 from diffblue/fix-regression-cbmc-memcpy1 506faf0 Refactor a function for base existence 617d388 Utility functions for generic types c07e6ca Update generic specialization map when replacing pointers ed26d0a Merge pull request diffblue#2058 from peterschrammel/stable-disjuncts b663734 Simplify extractbits(concatenation(...)) b091560 Typing and refactoring of simplify_extractbits 49ad1bd Merge pull request diffblue#974 from tautschnig/fix-assert-encoding 16e9599 Merge pull request diffblue#2063 from tautschnig/has-subtype 950f58b Merge pull request diffblue#2060 from tautschnig/opt-local-map 4222a94 Regression tests for unstable instanceof and virtual method disjuncts b44589e Make disjuncts in instanceof removal independent of class loading 3afff86 Make disjuncts in virtual method removal independent of class loading a385d9b Allowed split_string to be used on whitespace if not also trying to strip fe4a642 Merge pull request diffblue#2062 from tautschnig/no-has-deref 145f474 Adding tests for empty string edge cases 07009d4 Refactored test to run all combinations 252c24c Migrate old string utils unit tests e87edbf Removing wrong elements from the make file b165c52 make test work on 32-bit Linux b804570 Merge pull request diffblue#2048 from JohnDumbell/improvement/adding_null_object_id 61f14d8 Merge pull request diffblue#1962 from owen-jones-diffblue/owen-jones-diffblue/simplify-replace-java-nondet fdee7e8 Merge pull request diffblue#2059 from tautschnig/generalise-test 4625cc5 Extend global has_subexpr to take a predicate as has_subtype does e9ebd59 has_subtype(type, pred, ns) to search for contained type matching pred 1f1f67f Merge pull request diffblue#1889 from hannes-steffenhagen-diffblue/develop-feature_generate_function_bodies 048c188 Add unit test for java_replace_nondet 0fe48c9 Make remove_java_nondet work before remove_returns bcc4dc4 Use byte_extract_exprt constructor a1814a3 Get rid of thin (and duplicate) has_dereference wrapper 4122a28 Test to demonstrate assert bug on alpine d44bfd3 Also simplify assert structure found on alpine linux c5cde18 Do not generate redundant if statements in assert expansions 4fb0603 Make is_skip publicly available and use constant argument 5832ffd Negative numbers should also pass the test 3c23b28 Consistently disable simplify_exprt::local_replace_map da63652 Merge pull request diffblue#2054 from romainbrenguier/bugfix/clear-equations d77f6a2 Merge pull request diffblue#1831 from NathanJPhillips/feature/class-annotations 60c8296 Clear string_refinement equations (not dependencies) 314ed53 Correcting the value of ID_null_object 751a882 Factor out default CBMC options to static method 6f24009 Can now test for an option being set in optionst 9a8d937 Add to_annotated_type and enable type_checked_cast for annotated_typet ca77b4e Add test for added annotations b06a27d Introduce abstract qualifierst base class e6fb3bf Pretty printing of java_class_typet e22b95b Fix spurious virtual function related keywords 3ac6d17 Add type_dynamic_cast and friends for java_class_typet ce1f4d2 Add annotations to java_class_typet, its methods and fields f84753d Merge pull request diffblue#2042 from hannes-steffenhagen-diffblue/add_deprecate_macro 7a38669 Merge pull request diffblue#2017 from NathanJPhillips/feature/overlay-classes 75a4aec Revert "the deprecation will need to wait until codebase is clean" 67735b5 Disable deprecation warnings by default 0764f77 Merge pull request diffblue#2036 from romainbrenguier/id_array_list 690b606 Merge pull request diffblue#2039 from peterschrammel/fix-duplicate-msg-json-ui bba17d9 the deprecation will need to wait until codebase is clean 822c757 Regression test for redundant JSON message output de0644d Only force end of previous message if there actually is one. 5a637bf Merge pull request diffblue#2037 from hannes-steffenhagen-diffblue/add_deprecate_macro bff456a Merge pull request diffblue#2040 from tautschnig/remove-swp 87ebe90 Remove vim temp file 228c019 Fix duplicate message output in json-ui 0a2c43e Add DEPRECATED to functions documented as \deprecated 47f4b35 interval_sparse_arrayt constructor from array-list 026c4ca Declare an array_list_exprt class 50a2696 Define ID_array_list 513b67a Merge pull request diffblue#2038 from romainbrenguier/bugfix/assign-at-malloc-site c207106 Test with array of strings 60183a3 Assign string at malloc site 116fffd Add DEPRECATED macro to mark deprecated functions and variables 7952f2c Add option to generate function body to goto-instrument 3d4183a Add ability to overlay classes with new definitions of existing methods dbc2f71 Improve code and error message in infer_opaque_type_fields 7c0ea4d Tidied up java_class_loader_limitt git-subtree-dir: cbmc git-subtree-split: ad62682

majakusber requested review from cesaro, kroening, mgudemann, peterschrammel, smowton, tautschnig and thk123 as code owners April 12, 2018 16:47

majakusber force-pushed the generics_tg2717 branch from 982f69c to fdcff6e Compare April 13, 2018 10:02

tautschnig reviewed Apr 13, 2018

View reviewed changes

thk123 reviewed Apr 16, 2018

View reviewed changes

thomasspriggs approved these changes Apr 16, 2018

View reviewed changes

majakusber force-pushed the generics_tg2717 branch from db1421f to c075086 Compare April 16, 2018 15:53

thk123 approved these changes Apr 16, 2018

View reviewed changes

svorenova added 3 commits April 17, 2018 09:29

Update generic specialization map when replacing pointers

c07e6ca

When initiliazing a pointer and it is replaced by a different pointer due to inheritence make sure that the generic parameter specialization map is updated too.

Utility functions for generic types

617d388

These will be used in test generator to convert generic pointer types.

Refactor a function for base existence

506faf0

Now simply calls a function for base retrieval

majakusber force-pushed the generics_tg2717 branch from c075086 to 506faf0 Compare April 17, 2018 08:30

peterschrammel approved these changes Apr 17, 2018

View reviewed changes

thk123 merged commit 00b9bf6 into diffblue:develop Apr 17, 2018

majakusber deleted the generics_tg2717 branch April 18, 2018 16:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TG-2717] Generic inheritance with pointer replacement #2051

[TG-2717] Generic inheritance with pointer replacement #2051

majakusber commented Apr 12, 2018 •

edited

Loading

tautschnig Apr 13, 2018

majakusber Apr 16, 2018

tautschnig Apr 16, 2018

thk123 Apr 16, 2018

majakusber Apr 16, 2018

thk123 Apr 16, 2018

majakusber Apr 16, 2018

thomasspriggs left a comment

thomasspriggs Apr 16, 2018

majakusber commented Apr 16, 2018

thk123 commented Apr 16, 2018

majakusber commented Apr 16, 2018

thk123 left a comment

majakusber commented Apr 17, 2018 •

edited

Loading

majakusber commented Apr 17, 2018

peterschrammel left a comment

[TG-2717] Generic inheritance with pointer replacement #2051

[TG-2717] Generic inheritance with pointer replacement #2051

Conversation

majakusber commented Apr 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasspriggs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majakusber commented Apr 16, 2018

thk123 commented Apr 16, 2018

majakusber commented Apr 16, 2018

thk123 left a comment

Choose a reason for hiding this comment

majakusber commented Apr 17, 2018 • edited Loading

majakusber commented Apr 17, 2018

peterschrammel left a comment

Choose a reason for hiding this comment

majakusber commented Apr 12, 2018 •

edited

Loading

majakusber commented Apr 17, 2018 •

edited

Loading