Use static_cast instead of C-style casts for non-fundamental types #10498

1. 
   practicalswift commented at 12:35 pm on June 1, 2017: contributor

   A C-style cast is equivalent to try casting in the following order:

   1. const_cast(...)
   2. static_cast(...)
   3. const_cast(static_cast(...))
   4. reinterpret_cast(...)
   5. const_cast(reinterpret_cast(...))

   By using static_cast<T>(...) explicitly we avoid the possibility of an unintentional and dangerous reinterpret_cast. Furthermore static_cast<T>(...) allows for easier grepping of casts.

   For a more thorough discussion, see “ES.49: If you must use a cast, use a named cast” in the C++ Core Guidelines (Stroustrup & Sutter).

2. fanquake added the label Refactoring on Jun 1, 2017
3. in src/net_processing.cpp:2632 in 1a3c1e24cb outdated

   2628@@ -2629,7 +2629,7 @@ static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman& connman)
   2629     CNodeState &state = *State(pnode->GetId());
   2630 
   2631     BOOST_FOREACH(const CBlockReject& reject, state.rejects) {
   2632-        connman.PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, (std::string)NetMsgType::BLOCK, reject.chRejectCode, reject.strRejectReason, reject.hashBlock));
   2633+        connman.PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, static_cast<std::string>(NetMsgType::BLOCK), reject.chRejectCode, reject.strRejectReason, reject.hashBlock));
   

   ---

   ---

   

   benma commented at 12:53 pm on June 1, 2017:

   maybe std::string(NetMsgType::BLOCK) is nicer here.
4. in src/primitives/block.h:89 in 1a3c1e24cb outdated

   85@@ -86,14 +86,14 @@ class CBlock : public CBlockHeader
   86     CBlock(const CBlockHeader &header)
   87     {
   88         SetNull();
   89-        *((CBlockHeader*)this) = header;
   90+        *(static_cast<CBlockHeader*>(this)) = header;
   

   ---

   ---

   

   benma commented at 12:58 pm on June 1, 2017:

   Why doesn’t this need a cont_cast? this is const, isn’t it?

   ---

   

   practicalswift commented at 9:12 pm on June 4, 2017:

   @benma this isn’t const in the constructor:

   The type of this in a member function of class X is X* (pointer to X). If the member function is cv-qualified, the type of this is cv X* (pointer to identically cv-qualified X). Since constructors and destructors cannot be cv-qualified, the type of this in them is always X*, even when constructing or destroying a const object. (cppreference.com)

   ---

   

   benma commented at 9:20 pm on June 4, 2017:

   Of course, this is only const if the function is const. My bad.
5. benma approved
6. 
   benma commented at 1:03 pm on June 1, 2017: none

   utACK 1a3c1e24cbce60910843f434dc6c489d03eaed1d
7. practicalswift force-pushed on Jun 1, 2017
8. practicalswift force-pushed on Jun 1, 2017
9. 
   benma commented at 9:28 pm on June 4, 2017: none

   utACK 6090e51b5d3f8024197a31e6a5a50b812ba10aca
10. 
    MarcoFalke commented at 4:02 pm on June 5, 2017: member

    utACK 6090e51b5d3f8024197a31e6a5a50b812ba10aca does not change the binariy on my platform
11. practicalswift force-pushed on Jun 18, 2017
12. practicalswift force-pushed on Jul 15, 2017
13. practicalswift force-pushed on Jul 17, 2017
14. 
    practicalswift commented at 6:14 pm on July 17, 2017: contributor

    Rebased!
15. practicalswift force-pushed on Aug 14, 2017
16. 
    practicalswift commented at 3:44 pm on August 14, 2017: contributor

    Rebased!
17. practicalswift force-pushed on Sep 10, 2017
18. 
    practicalswift commented at 7:06 pm on September 10, 2017: contributor

    Rebased! :-)
19. practicalswift force-pushed on Sep 21, 2017
20. practicalswift force-pushed on Sep 21, 2017
21. Use static_cast instead of C-style casts for non-fundamental types

    A C-style cast is equivalent to try casting in the following order:
    
    1. const_cast(...)
    2. static_cast(...)
    3. const_cast(static_cast(...))
    4. reinterpret_cast(...)
    5. const_cast(reinterpret_cast(...))
    
    By using static_cast<T>(...) explicitly we avoid the possibility
    of an unintentional and dangerous reinterpret_cast. Furthermore
    static_cast<T>(...) allows for easier grepping of casts.

    9ad6746ccd
22. practicalswift force-pushed on Sep 22, 2017
23. 
    practicalswift commented at 2:05 pm on September 22, 2017: contributor

    Rebased!
24. 
    practicalswift commented at 12:45 pm on January 31, 2018: contributor

    Anyone willing to review? ACK or NACK? :-)
25. 
    ghost commented at 5:34 pm on January 31, 2018: none

    Why not go the extra mile and also add -Wold-style-cast, otherwise you’ll be back fixing it a week later.
26. 
    ryanofsky commented at 5:53 pm on January 31, 2018: member

    utACK 9ad6746ccd6dc31141fd0144686b641e31bf626b
27. 
    MarcoFalke commented at 6:07 pm on January 31, 2018: member

    re-reviewed

     0-----BEGIN PGP SIGNED MESSAGE-----
     1Hash: SHA512
     2
     3re-utACK 9ad6746ccd6dc31141fd0144686b641e31bf626b
     4-----BEGIN PGP SIGNATURE-----
     5
     6iQIcBAEBCgAGBQJacgWeAAoJENLqSFDnUoslDH0P/0t30sv4UootDVMUutXKfnsK
     7mHKRDOKMbWLYQonJ93x89Oll+U+r+B7m7CzlxqxUNI2Zd6DETOprwLAG8F1yKHez
     8JUjjndpfP/6wgzyv+YEJPehFL2vVSu3Di0j82qk/1vSaYHA94bh6Y+pRnVE65951
     9YdpuD0scPfXIMuR93swiGpw8ttFEqZVcGtjzWlwgKO23cdJaIlZatmpi1Mw/eQea
    10pqhol7AgtzZeEnISYDRVcObPxVzUabGDkVXUjafJK14enMLpg6ddjV4tldLCtVfR
    11J5yEfxGGTcYLm5TYpvYGQS58GWZSdbUm22HPDCcjYZW+NVvXRcvsaIYPE+BZJ/u/
    12q9jEs3wAP7lhTp0XiFmJ6UFlyrkVBopGmNI1KPWtsxdvjoouAH8XpO3RsB6/pNZq
    13nLZcvg/QNSbJ5ypMFshcZ2iJjVtgmIuwC/kceAuy3nK0EkK9BFUd9qS3dzdAbYiZ
    14aYfZjn6fu0PWxItMzEMActleYwAdhQLjc6tX35tSlVE0ZOSJx0FDWi6dIPyzlX6L
    15jWGKpMCm2m/Xt19av4IS9AHk+xoIGsQrgaWt+7TYc7vssR/dlw8IXsKI2D68AEJE
    16IXxh6HGNeBu/2cbTQ4bwA4CtAaE5sS44EdbxbduF1mAPhUJEoywP51F9VkNAqaQQ
    1764GCcFS6tzqUfliIrLGl
    18=Gwy1
    19-----END PGP SIGNATURE-----
    

28. 
    practicalswift commented at 8:33 am on February 1, 2018: contributor

    @kekimusmaximus That would be nice but I think there might be some instances of C-style casts for non-fundamental types in our dependencies, no? :-)
29. 
    ryanofsky commented at 9:10 pm on February 7, 2018: member

    Seems like this could be merged: obvious change, has 3 acks.
30. MarcoFalke merged this on Feb 7, 2018
31. MarcoFalke closed this on Feb 7, 2018

    ---

32. MarcoFalke referenced this in commit 0277173b1d on Feb 7, 2018
33. in src/qt/bitcoin.cpp:712 in 9ad6746ccd

    708@@ -709,7 +709,7 @@ int main(int argc, char *argv[])
    709         if (BitcoinCore::baseInitialize()) {
    710             app.requestInitialize();
    711 #if defined(Q_OS_WIN) && QT_VERSION >= 0x050000
    712-            WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely...").arg(QObject::tr(PACKAGE_NAME)), (HWND)app.getMainWinId());
    713+            WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely...").arg(QObject::tr(PACKAGE_NAME)), static_cast<HWND>(app.getMainWinId()));
    

    ---

    ---

    

    Sjors commented at 9:01 pm on February 10, 2018:

    Maybe unrelated, but I’m trying to make a Gitian Windows build for a branch based off of master and it throws:

    0qt/bitcoin.cpp: In function ‘int main(int, char**)’:
    1qt/bitcoin.cpp:709:173: error: invalid static_cast from type ‘WId {aka unsigned int}’ to type ‘HWND’
    2             WinShutdownMonitor::registerShutdownBlockReason(QObject::tr("%1 didn't yet exit safely...").arg(QObject::tr(PACKAGE_NAME)), static_cast<HWND>(app.getMainWinId()));                                                                                                                                                                       ^
    

    ---

    

    Sjors commented at 9:09 pm on February 10, 2018:

    Just noticed #12386…
34. practicalswift referenced this in commit f40df29d96 on Feb 12, 2018
35. MarcoFalke referenced this in commit c997f88082 on Feb 12, 2018
36. esotericnonsense referenced this in commit d4a24b5471 on Feb 19, 2018
37. Willtech referenced this in commit a3fa7055aa on Feb 23, 2018
38. HashUnlimited referenced this in commit d7183ebaf6 on Jul 31, 2018
39. lionello referenced this in commit 8b245ac953 on Nov 7, 2018
40. jasonbcox referenced this in commit 256ef1e899 on Apr 5, 2019
41. jonspock referenced this in commit b0ce0d03b4 on Apr 7, 2019
42. jonspock referenced this in commit 51aa76af4c on Apr 8, 2019
43. jonspock referenced this in commit e6f978735f on Apr 8, 2019
44. jonspock referenced this in commit e41df7d91d on Apr 8, 2019
45. PastaPastaPasta referenced this in commit a1de165dbe on Jun 9, 2020
46. PastaPastaPasta referenced this in commit d51b037fe4 on Jun 9, 2020
47. PastaPastaPasta referenced this in commit e10ab64311 on Jun 10, 2020
48. PastaPastaPasta referenced this in commit a09681c84f on Jun 10, 2020
49. PastaPastaPasta referenced this in commit 8a1710c3dc on Jun 10, 2020
50. PastaPastaPasta referenced this in commit 26ca61ec21 on Jun 11, 2020
51. PastaPastaPasta referenced this in commit c7ef89ec3a on Jun 11, 2020
52. PastaPastaPasta referenced this in commit a6c210a755 on Jun 11, 2020
53. PastaPastaPasta referenced this in commit 13ca02c45f on Jun 11, 2020
54. PastaPastaPasta referenced this in commit e4f13ff508 on Jun 11, 2020
55. PastaPastaPasta referenced this in commit 1b4ba4fafd on Jun 11, 2020
56. PastaPastaPasta referenced this in commit b8bb2a7ab5 on Jun 12, 2020
57. random-zebra referenced this in commit 9232a36130 on Feb 13, 2021
58. practicalswift deleted the branch on Apr 10, 2021
59. gades referenced this in commit a0257555e7 on Feb 5, 2022
60. DrahtBot locked this on Aug 18, 2022

Contributors
practicalswift benma MarcoFalke ghost ryanofsky Sjors

Labels
Refactoring