Add deprecate macro #2037

hannes-steffenhagen-diffblue · 2018-04-10T14:34:05Z

We currently have some "deprecated" functions, but we don't emit any diagnostics when they are used.

This adds a DEPRECATED macro that uses compiler builtins (or, when available, the C++14 [[deprecated]] attribute) to mark functions as deprecated.

To facilitate a gradual replacement of deprecated functionality deprecation warnings are not treated as errors.

tautschnig

I think it's good to raise deprecation messages to compiler warnings. What I am worried about, however, is the existing deprecation markup for code that shouldn't even have seen any customer usage and thus really should just be changed. In my opinion, marking code as deprecated makes sense when there are external users that need to be warned so that they have time to migrate. In absence of such external users the code should either be changed or an issue to should be opened to do so at the earliest convenience.

tautschnig · 2018-04-10T14:57:24Z

src/solvers/refinement/string_constraint_generator_insert.cpp

@@ -130,6 +131,7 @@ exprt string_constraint_generatort::add_axioms_for_insert(
 /// \param f: function application with three arguments: a string, an
 ///   integer offset, and an integer
 /// \return an expression
+DEPRECATED("")


Can more details be provided? @romainbrenguier?

tautschnig · 2018-04-10T14:58:13Z

src/solvers/refinement/string_constraint_generator_insert.cpp

@@ -150,6 +152,7 @@ exprt string_constraint_generatort::add_axioms_for_insert_int(
 /// \param f: function application with three arguments: a string, an
 ///   integer offset, and a Boolean
 /// \return a new string expression
+DEPRECATED("should convert the value to string and call insert")


Am I right when saying that some bits are marked deprecated here when really they just need to get onto someone's task list to fix?

I mainly wrote this to make it easier to fix code that uses deprecated functionality as we go and to prevent people from writing new code with deprecated functionality, but if someone took the time to fix all of the instances where those are used that would of course also be helpful.

This is correctly marked deprecated but the message is not totally clear: instead of calling this function we can obtain the same result by using calls to add_axioms_from_boolean and add_axioms_for_insert.

tautschnig · 2018-04-10T14:58:51Z

src/util/arith_tools.h

@@ -14,17 +14,21 @@ Author: Daniel Kroening, [email protected]
 #include "optional.h"
 #include "invariant.h"

+#include <util/deprecation.h>


Use "deprecation.h" in this folder.

tautschnig · 2018-04-10T14:59:19Z

src/util/mp_arith.h

@@ -52,12 +53,15 @@ const std::string integer2binary(const mp_integer &, std::size_t width);
 const mp_integer binary2integer(const std::string &, bool is_signed);

 /// \deprecated use numeric_cast<unsigned long long> instead
+DEPRECATED("Use numeric cast<unsigned long long> instead")


Missing _?

NlightNFotis

Just three minor notes. Otherwise looks solid as a PR, both in content and in size. Good work.

NlightNFotis · 2018-04-10T15:32:00Z

src/util/mp_arith.h

 mp_integer::ullong_t integer2ulong(const mp_integer &);

 /// \deprecated use numeric_cast<std::size_t> instead
+DEPRECATED("Use numeric cast<std::size_t> instead")


This should be numeric_cast<std::size_t>

NlightNFotis · 2018-04-10T15:32:10Z

src/util/mp_arith.h

 std::size_t integer2size_t(const mp_integer &);

 /// \deprecated use numeric_cast<unsigned> instead
+DEPRECATED("Use numeric cast<unsigned> instead")


NlightNFotis · 2018-04-10T15:34:51Z

src/util/deprecation.h

+
+\*******************************************************************/
+
+#ifndef CPROVER_UTIL_DEPRECATION_H


I think this file would be better named deprecate.h, as most other things in the folder are named as verbs and not nouns. But it's a minor preference of mine to make it more consistent with the rest of the files.

peterschrammel · 2018-04-10T18:05:11Z

Btw, the list of all functions marked deprecated in their doxygen header is here: http://cprover.diffblue.com/deprecated.html

romainbrenguier · 2018-04-10T14:46:10Z

src/solvers/refinement/string_constraint_generator_insert.cpp

@@ -130,6 +131,7 @@ exprt string_constraint_generatort::add_axioms_for_insert(
 /// \param f: function application with three arguments: a string, an
 ///   integer offset, and an integer
 /// \return an expression
+DEPRECATED("")


Can you copy the same message as the over DEPRECATED from this file?

romainbrenguier · 2018-04-10T14:48:37Z

src/solvers/refinement/string_constraint_generator_valueof.cpp

@@ -72,6 +75,7 @@ exprt string_constraint_generatort::add_axioms_from_bool(
 /// \param res: string expression for the result
 /// \param b: Boolean expression
 /// \return code 0 on success
+DEPRECATED("This is language dependent")


Maybe put the same message for both add_axioms_from_bool implementations, something like "This is Java specific and should be implemented in a Java model instead."

kroening · 2018-04-11T12:40:12Z

Please revert, this makes too much noise.

kroening · 2018-04-11T12:40:52Z

Deprecation is something that happens in timescales of years, and certainly the codebase itself must be clear of warnings before this is turned on.

tautschnig · 2018-04-11T12:58:50Z

Instead of reverting, may I suggest to change the Wno-error= to Wno-?!

hannes-steffenhagen-diffblue · 2018-04-11T13:05:15Z

So we can:

Revert this entirely
Turn off deprecation warnings by default on all builds (travis + local)
Turn them off for travis builds only
Turn the warnings off by default and enable them for one travis config so we can still see if we're still using deprecated code somewhere

tautschnig · 2018-04-11T13:31:02Z

I'd lean towards 4, but then I also approved the original PR so I'm biased.

chrisr-diffblue · 2018-04-11T13:33:30Z

I'd probably also lean towards 4 as well. It would be a shame to see this PR just reverted, as I do think there is some value to it in the long term .

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

hannes-steffenhagen-diffblue requested review from chrisr-diffblue, kroening, martin-cs, peterschrammel, romainbrenguier, smowton and tautschnig as code owners April 10, 2018 14:34

hannes-steffenhagen-diffblue force-pushed the add_deprecate_macro branch from e26a75e to 1157b2f Compare April 10, 2018 14:39

hannes-steffenhagen-diffblue self-assigned this Apr 10, 2018

tautschnig approved these changes Apr 10, 2018

View reviewed changes

hannes-steffenhagen-diffblue force-pushed the add_deprecate_macro branch from 1157b2f to 10429d7 Compare April 10, 2018 15:15

NlightNFotis approved these changes Apr 10, 2018

View reviewed changes

Add DEPRECATED macro to mark deprecated functions and variables

116fffd

hannes-steffenhagen-diffblue force-pushed the add_deprecate_macro branch from 10429d7 to 9c601a5 Compare April 10, 2018 15:56

peterschrammel approved these changes Apr 11, 2018

View reviewed changes

romainbrenguier approved these changes Apr 11, 2018

View reviewed changes

Add DEPRECATED to functions documented as \deprecated

0a2c43e

hannes-steffenhagen-diffblue force-pushed the add_deprecate_macro branch from 9c601a5 to 0a2c43e Compare April 11, 2018 10:00

hannes-steffenhagen-diffblue merged commit 5a637bf into diffblue:develop Apr 11, 2018

hannes-steffenhagen-diffblue mentioned this pull request Apr 11, 2018

Disable deprecation warnings by default #2042

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add deprecate macro #2037

Add deprecate macro #2037

hannes-steffenhagen-diffblue commented Apr 10, 2018

tautschnig left a comment

tautschnig Apr 10, 2018

tautschnig Apr 10, 2018

hannes-steffenhagen-diffblue Apr 10, 2018

romainbrenguier Apr 11, 2018

tautschnig Apr 10, 2018

tautschnig Apr 10, 2018

NlightNFotis left a comment

NlightNFotis Apr 10, 2018

NlightNFotis Apr 10, 2018

NlightNFotis Apr 10, 2018

peterschrammel commented Apr 10, 2018

romainbrenguier Apr 10, 2018

romainbrenguier Apr 10, 2018

kroening commented Apr 11, 2018

kroening commented Apr 11, 2018

tautschnig commented Apr 11, 2018

hannes-steffenhagen-diffblue commented Apr 11, 2018 •

edited

Loading

tautschnig commented Apr 11, 2018

chrisr-diffblue commented Apr 11, 2018


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

		#ifndef CPROVER_UTIL_DEPRECATION_H

Add deprecate macro #2037

Add deprecate macro #2037

Conversation

hannes-steffenhagen-diffblue commented Apr 10, 2018

tautschnig left a 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

Choose a reason for hiding this comment

NlightNFotis left a 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

peterschrammel commented Apr 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kroening commented Apr 11, 2018

kroening commented Apr 11, 2018

tautschnig commented Apr 11, 2018

hannes-steffenhagen-diffblue commented Apr 11, 2018 • edited Loading

tautschnig commented Apr 11, 2018

chrisr-diffblue commented Apr 11, 2018

hannes-steffenhagen-diffblue commented Apr 11, 2018 •

edited

Loading