Don't use C style casts #363

laudrup · 2016-11-23T00:40:59Z

Change C style casts to C++ static casts.

This makes the code compile with the -Wold-style-cast warning enabled

Cereal uses fairly clean modern C++ style, but unfortunately a few headers use C style casts meaning they cannot be used without disabling warnings about these casts, using #pragmas or similar.

This shouldn't introduce any functional changes.

Change C style casts to C++ static casts. This makes the code compile with the -Wold-style-cast warning enabled

erichkeane · 2016-11-23T07:30:50Z

include/cereal/details/polymorphic_impl.hpp

@@ -645,7 +645,7 @@ namespace cereal
      //! Binding for non abstract types
      void bind(std::false_type) const
 	    {
-		    instantiate_polymorphic_binding((T*) 0, 0, Tag{}, adl_tag{});
+                   instantiate_polymorphic_binding(static_cast<T*>(0), 0, Tag{}, adl_tag{});


Almost definitely should be nullptr.

erichkeane · 2016-11-23T07:33:00Z

include/cereal/external/base64.hpp

@@ -95,7 +95,7 @@ namespace cereal
        char_array_4[i++] = encoded_string[in_]; in_++;
        if (i ==4) {
          for (i = 0; i <4; i++)
-            char_array_4[i] = (unsigned char) chars.find( char_array_4[i] );
+            char_array_4[i] = static_cast<unsigned char>(chars.find( char_array_4[i] ));


Is there a good reason for char_array* to be unsigned char, particularly when the string is a std::string. alternatively, chars should probably be a fixed array, right?

erichkeane · 2016-11-23T07:33:48Z

sandbox/sandbox.cpp

@@ -651,9 +651,9 @@ int main()
    iar( d1 );
    assert( d1->x == 4 && d1->y == 3 );
    iar( d2 );
-    assert( ((Derived*)d2.get())->x == 5 && ((Derived*)d2.get())->y == 4 );
+    assert( static_cast<Derived*>(d2.get())->x == 5 && static_cast<Derived*>(d2.get())->y == 4 );


Dynamic cast + a check?

erichkeane · 2016-11-23T07:35:11Z

sandbox/sandbox_json.cpp

 }

 template <class Archive>
 void load( Archive & ar, Bla & b )
 {
-  ar( (int&)b );
+  ar( static_cast<int&>(b) );


erichkeane · 2016-11-23T07:35:23Z

sandbox/sandbox_json.cpp

@@ -320,13 +320,13 @@ enum Bla
 template <class Archive>
 void save( Archive & ar, Bla const & b )
 {
-  ar( (const int &)b );
+  ar( static_cast<const int &>(b) );


I don't totally see what is happening here, but is the cast totally necessary? Should this cast be to std::underlying_type?

I think a lot of the code in the sandbox files was temporary for debugging development, especially early on in cereal. Most tests of note are put into actual test cases now.

erichkeane · 2016-11-23T07:36:04Z

unittests/pod.cpp

@@ -132,7 +132,7 @@ void test_pod()
    BOOST_CHECK_EQUAL(i_int32  , o_int32);
    BOOST_CHECK_EQUAL(i_uint64 , o_uint64);
    BOOST_CHECK_EQUAL(i_int64  , o_int64);
-    BOOST_CHECK_CLOSE(i_float  , o_float,  (float)1e-5);
+    BOOST_CHECK_CLOSE(i_float  , o_float,  static_cast<float>(1e-5));


erichkeane · 2016-11-23T07:36:42Z

unittests/portable_binary_archive.cpp

@@ -72,7 +72,7 @@ inline void swapBytes( T & t )
    BOOST_CHECK_EQUAL(i_int32  , o_int32);               \
    BOOST_CHECK_EQUAL(i_uint64 , o_uint64);              \
    BOOST_CHECK_EQUAL(i_int64  , o_int64);               \
-    if( !std::isnan(i_float) && !std::isnan(o_float) ) BOOST_CHECK_CLOSE(i_float  , o_float,  (float)1e-5); \
+    if( !std::isnan(i_float) && !std::isnan(o_float) ) BOOST_CHECK_CLOSE(i_float  , o_float,  static_cast<float>(1e-5)); \


erichkeane · 2016-11-23T07:37:01Z

unittests/structs_specialized.cpp

@@ -441,7 +441,7 @@ void test_structs_specialized()
    BOOST_CHECK(i_ispl.x == o_ispl.x);
    BOOST_CHECK(i_isplv.x == o_isplv.x);

-    BOOST_CHECK_EQUAL(((SpecializedMSplitPolymorphic*)i_shared_ispl.get())->x, ((SpecializedMSplitPolymorphic*)o_shared_ispl.get())->x);
+    BOOST_CHECK_EQUAL(static_cast<SpecializedMSplitPolymorphic*>(i_shared_ispl.get())->x, static_cast<SpecializedMSplitPolymorphic*>(o_shared_ispl.get())->x);


Dynamic + check?

erichkeane · 2016-11-23T07:37:15Z

unittests/polymorphic.cpp

-    BOOST_CHECK_EQUAL(*((PolyDerived*)i_shared.get()), *((PolyDerived*)i_locked.get()));
-    BOOST_CHECK_EQUAL(*((PolyDerived*)i_locked.get()), *((PolyDerived*)o_locked.get()));
-    BOOST_CHECK_EQUAL(*((PolyDerived*)i_unique.get()), *((PolyDerived*)o_unique.get()));
+    BOOST_CHECK_EQUAL(*static_cast<PolyDerived*>(i_shared.get()), *static_cast<PolyDerived*>(o_shared.get()));


Dynamic + check for all?

laudrup · 2016-11-23T10:05:59Z

@erichkeane

Very reasonable comments.

All I did in this commit was to "blindly" change the C casts to static_casts. I didn't want to risk changing any functionality without a better understanding of the code.

I cannot see that this commit introduces any of the issues you have commented on. Am I missing something?

Thanks.

kklouzal · 2016-11-23T10:30:20Z

@laudrup You're missing the fact that he did exactly as you wanted and changed all the C-Style Casts into C++ static_cast's 👍 👍

laudrup · 2016-11-23T12:02:57Z

@kklouzal

One of us is confused. I was the one who changed the C-style casts to C++ static_casts. @erichkeane came with the very good points about how to improve it. :-)

erichkeane · 2016-11-23T16:43:28Z

I'm not maintainer, so this obviously isn't up to me :)

Just thought @AzothAmmo might find my suggestions useful. A vast majority of these are in unittest/sandbox, so they don't really matter that much anyway

AzothAmmo · 2016-11-23T17:42:15Z

I'll work these in taking @erichkeane's comments into account.

erichkeane · 2016-11-23T17:47:05Z

👍

laudrup · 2016-11-23T17:48:18Z

@AzothAmmo Cool. Thanks a lot. Let me know if I can help in any way, but as I wrote I'm not too familiar with this code so I'd rather avoid getting in the way.

see #363

AzothAmmo · 2016-11-28T19:54:22Z

Modified with most of the suggestions of @erichkeane, didn't bother with checks on the dynamic casts since they'll fail out the unit tests either way if they don't work.

laudrup · 2016-11-28T20:36:14Z

Nice!

Thanks a lot for this.

commit aa891a4 Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Jun 26 13:42:08 2017 -0700 Resolves USCiLab#414 commit fcef0da Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri May 5 11:37:12 2017 -0700 Fix shadowing issue for USCiLab#401, recent osx compile issue re: USCiLab#354 commit 950aca4 Merge: ad90557 35a36af Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri May 5 11:09:34 2017 -0700 Merge branch 'hoensr-xml-no-size-attributes' into develop commit 35a36af Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri May 5 10:54:25 2017 -0700 Standardize interface for options (xml) see USCiLab#401 commit ad90557 Merge: f031131 2ab15f7 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed May 3 11:39:02 2017 -0700 Merge branch 'develop' of github.com:USCiLab/cereal into develop commit f031131 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed May 3 11:38:35 2017 -0700 modifications for g47 and comment out memory intensive testing commit c4dcc8d Merge: 68f56ee 676d329 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed May 3 11:11:08 2017 -0700 Merge branch 'issue_354' into develop still needs more work but this is an improvement for now see USCiLab#354 commit 52b03d5 Author: Robin Hoens <robin.hoens@qiagen.com> Date: Mon Apr 10 17:13:39 2017 +0200 Add option to turn off the size=dynamic attributes of lists commit 2ab15f7 Merge: 68f56ee 7723503 Author: Shane Grant <w.shane.grant@gmail.com> Date: Tue Apr 11 16:17:51 2017 -0700 Merge pull request USCiLab#397 from albertziegenhagel/issue_396 Add remove_reference to determine whether template argument to BinaryData is const commit 7723503 Author: Albert Ziegenhagel <albert.ziegenhagel@scai.fraunhofer.de> Date: Tue Apr 11 10:32:55 2017 +0200 Add remove_reference to determine whether template argument to BinaryData is const commit 68f56ee Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Apr 10 11:22:38 2017 -0700 Remove undefined behavior, see USCiLab#390 commit 676d329 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed Feb 22 14:43:31 2017 -0800 adventures in microoptimization USCiLab#354 commit 546fd9b Author: Shane Grant <w.shane.grant@gmail.com> Date: Sun Feb 19 16:25:32 2017 -0800 tinkering on USCiLab#354 commit 8b8f581 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed Feb 15 13:39:42 2017 -0800 Fix macro for double comparison in unit test relates USCiLab#338 commit 51cbda5 Merge: 70c4420 e38d6fe Author: Shane Grant <w.shane.grant@gmail.com> Date: Sun Feb 12 14:06:58 2017 -0800 Merge branch 'develop' for release 1.2.2 commit e38d6fe Author: Shane Grant <w.shane.grant@gmail.com> Date: Sun Feb 12 13:46:25 2017 -0800 fix update doc script commit 70c4420 Author: Shane Grant <w.shane.grant@gmail.com> Date: Sun Feb 12 00:25:30 2017 -0800 Update README.md add appveyor badge commit 2590f21 Author: Shane Grant <w.shane.grant@gmail.com> Date: Sat Feb 11 23:47:00 2017 -0800 Properly use multimap for lookup in poly casting relates USCiLab#356, still need final testing on MSVC commit a917374 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed Feb 8 10:47:02 2017 -0800 size_type now specified by macro CEREAL_SIZE_TYPE resolves USCiLab#379 commit ee17db5 Merge: b827b95 f577fc4 Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Feb 6 22:51:29 2017 -0800 Merge branch 'develop' of github.com:USCiLab/cereal into develop commit b827b95 Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Feb 6 22:50:56 2017 -0800 Fixes need for special MSVC case, see USCiLab#373 commit f577fc4 Author: Shane Grant <w.shane.grant@gmail.com> Date: Sun Feb 5 22:13:08 2017 -0800 Turn on warnings as errors for MSVC, warning level to 3 Can't do level 4 warnings yet - need to make an upstream change to doctest commit fb6606d Author: Shane Grant <w.shane.grant@gmail.com> Date: Sun Feb 5 18:13:37 2017 -0800 Do not build coverage or valgrind for MSVC commit a2d5a15 Merge: 4a92e29 e4d543d Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri Jan 27 10:29:51 2017 -0800 Merge branch 'tusharpm-develop' into develop see USCiLab#373 Still need to address why windows needed a modifcation to polymorphic test to compile commit e4d543d Author: Tushar Maheshwari <tushar27192@gmail.com> Date: Thu Jan 26 16:53:47 2017 +0530 Fix merge issues commit 2261fee Author: Tushar Maheshwari <tushar27192@gmail.com> Date: Sun Dec 18 17:32:04 2016 +0530 Pull requests to not increment build numbers commit 4ff4db8 Author: Tushar Maheshwari <tushar27192@gmail.com> Date: Sun Dec 11 20:03:02 2016 +0530 boost new version commit 655696a Author: Tushar Maheshwari <tushar27192@gmail.com> Date: Sun Dec 11 15:42:00 2016 +0530 AppVeyor integration commit df44243 Author: Tushar Maheshwari <tushar27192@gmail.com> Date: Sun Dec 11 01:01:10 2016 +0530 Enable cross-platform portability test CMake fix 32-bit executable with generator Win64 commit 0a908bc Author: Tushar Maheshwari <tushar27192@gmail.com> Date: Sat Dec 10 17:27:28 2016 +0530 Make tests pass with Windows commit 4a92e29 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed Jan 25 11:04:24 2017 -0800 no longer need boost test in travis commit a8e9963 Merge: 75e50ee 1d67d44 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed Jan 25 10:56:38 2017 -0800 Merge branch 'develop_doctest' into develop see USCiLab#139 commit 75e50ee Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 13:48:46 2016 -0800 remove old code from sandbox relates USCiLab#363 commit 507f97d Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 11:53:56 2016 -0800 add -Wold-style-casts see USCiLab#363 commit f69ad7c Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 11:50:23 2016 -0800 tab to space commit d21b0c0 Merge: 29829c1 e63f08f Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 11:47:53 2016 -0800 Merge branch 'laudrup-master' into develop see USCiLab#363 commit e63f08f Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 11:47:24 2016 -0800 minor adjustments to casts, see comments on USCiLab#363 commit 29829c1 Merge: 9978e0c ad92746 Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 10:41:27 2016 -0800 Merge branch 'FlexCoreLib-doxygen_access_from_extern' into develop see USCiLab#365 commit ad92746 Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 10:41:02 2016 -0800 comment out obsolete doxygen commit 9978e0c Merge: 1edc5c6 6e71766 Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 10:37:17 2016 -0800 Merge pull request USCiLab#366 from headupinclouds/pr.is_loading.is_saving Add is_loading and is_saving values to cereal::{Input,Output}Archive commit 1edc5c6 Merge: 72d7936 6086234 Author: Shane Grant <w.shane.grant@gmail.com> Date: Mon Nov 28 10:35:24 2016 -0800 Merge pull request USCiLab#367 from tusharpm/develop Travis configuration updates commit 6086234 Author: Tushar Maheshwari <tushar27192@gmail.com> Date: Sun Nov 27 13:57:26 2016 +0530 Travis configuration updates - Switch to trusty container-based infrastructure for linux systems - Add a job to test with clang compiler (skips portability test) - Add a job to test on macOS with Xcode8 commit 6e71766 Author: David Hirvonen <dhirvonen@elucideye.com> Date: Sat Nov 26 12:31:56 2016 -0500 Remove spurious character commit 9376ca6 Author: Kasper Laudrup <laudrup@stacktrace.dk> Date: Wed Nov 23 01:34:53 2016 +0100 Don't use C style casts Change C style casts to C++ static casts. This makes the code compile with the -Wold-style-cast warning enabled commit 1d67d44 Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri Nov 4 16:07:00 2016 -0700 Removed boost_test as a requirement -Now build 32 bit unit tests if portability testing is enabled -No more boost+clang issues over std::string abi see USCiLab#139, USCiLab#123 commit 66528b6 Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri Nov 4 15:51:57 2016 -0700 last? conversions for USCiLab#139 commit 978b3b5 Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri Nov 4 12:11:51 2016 -0700 typo fix in comment USCiLab#139 commit 13ae560 Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri Nov 4 12:10:24 2016 -0700 more USCiLab#139 commit 07818f4 Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri Nov 4 12:00:16 2016 -0700 more USCiLab#139 commit b5e500d Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed Nov 2 16:32:35 2016 -0700 more USCiLab#139 commit a6e59d7 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed Nov 2 13:38:33 2016 -0700 more for USCiLab#139 commit cd46374 Author: Shane Grant <w.shane.grant@gmail.com> Date: Wed Nov 2 12:20:22 2016 -0700 more converted for USCiLab#139 commit 38e1548 Author: Shane Grant <w.shane.grant@gmail.com> Date: Tue Nov 1 11:59:40 2016 -0700 headers USCiLab#139 commit 15c7339 Author: Shane Grant <w.shane.grant@gmail.com> Date: Tue Nov 1 11:59:02 2016 -0700 more conversions USCiLab#139 commit 928cd36 Author: Shane Grant <w.shane.grant@gmail.com> Date: Tue Nov 1 11:41:28 2016 -0700 more conversions to CHECK_EQ see USCiLab#139 commit 0a262ec Author: Shane Grant <w.shane.grant@gmail.com> Date: Tue Nov 1 11:36:22 2016 -0700 More tests split, switch to CHECk_EQ over CHECK see USCiLab#139 commit 671999e Author: Shane Grant <w.shane.grant@gmail.com> Date: Fri Oct 28 14:28:37 2016 -0700 Initial progress on removing boost test and moving to doctest to better support modules relates USCiLab#123 commit a066808 Author: Caspar Kielwein <Caspar@Kielwein.de> Date: Sun Oct 2 16:09:04 2016 +0200 Add cereal.doxytags as tagfile to allow external projects to link to cereal documentation. commit 3fb59db Author: Caspar Kielwein <Caspar@Kielwein.de> Date: Sun Oct 2 15:53:28 2016 +0200 Build doxygen documentation with separate CMakeLists. Extracted CMakelists.txt to doc subdirectory. Changed paths in doxyfile.in and CMakeLists accordingly. added doc as subdirectory in main CMakeLists.txt.

Don't use C style casts

9376ca6

Change C style casts to C++ static casts. This makes the code compile with the -Wold-style-cast warning enabled

erichkeane reviewed Nov 23, 2016

View reviewed changes

AzothAmmo added the enhancement label Nov 23, 2016

AzothAmmo added this to the v1.2.3 milestone Nov 23, 2016

AzothAmmo modified the milestones: v1.2.2, v1.2.3 Nov 28, 2016

AzothAmmo added a commit that referenced this pull request Nov 28, 2016

minor adjustments to casts, see comments on #363

e63f08f

AzothAmmo added a commit that referenced this pull request Nov 28, 2016

Merge branch 'laudrup-master' into develop

d21b0c0

see #363

AzothAmmo added a commit that referenced this pull request Nov 28, 2016

add -Wold-style-casts see #363

507f97d

AzothAmmo closed this Nov 28, 2016

AzothAmmo added a commit that referenced this pull request Nov 28, 2016

remove old code from sandbox relates #363

75e50ee

erichkeane mentioned this pull request Nov 29, 2016

Using C++11 static_cast instead of C-like casts. #369

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't use C style casts #363

Don't use C style casts #363

laudrup commented Nov 23, 2016

erichkeane Nov 23, 2016

erichkeane Nov 23, 2016

erichkeane Nov 23, 2016

erichkeane Nov 23, 2016

erichkeane Nov 23, 2016

AzothAmmo Nov 28, 2016

erichkeane Nov 23, 2016

erichkeane Nov 23, 2016

erichkeane Nov 23, 2016

erichkeane Nov 23, 2016

laudrup commented Nov 23, 2016

kklouzal commented Nov 23, 2016

laudrup commented Nov 23, 2016 •

edited

Loading

erichkeane commented Nov 23, 2016

AzothAmmo commented Nov 23, 2016

erichkeane commented Nov 23, 2016

laudrup commented Nov 23, 2016

AzothAmmo commented Nov 28, 2016

laudrup commented Nov 28, 2016

Don't use C style casts #363

Don't use C style casts #363

Conversation

laudrup commented Nov 23, 2016

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laudrup commented Nov 23, 2016

kklouzal commented Nov 23, 2016

laudrup commented Nov 23, 2016 • edited Loading

erichkeane commented Nov 23, 2016

AzothAmmo commented Nov 23, 2016

erichkeane commented Nov 23, 2016

laudrup commented Nov 23, 2016

AzothAmmo commented Nov 28, 2016

laudrup commented Nov 28, 2016

laudrup commented Nov 23, 2016 •

edited

Loading