scripted-diff: Use <cxxx> instead of deprecated <xxx.h> when including C compatibility headers #11261

practicalswift commented at 11:00 PM on September 6, 2017: contributor

Use <cxxx> instead of deprecated <xxx.h> when including C compatibility headers.

Includes changed:

<assert.h>
<errno.h>
<limits.h>
<math.h>
<signal.h>
<stdarg.h>
<stdint.h>
<stdio.h>
<stdlib.h>
<string.h>

laanwj commented at 11:03 PM on September 6, 2017: member

Another huge diff. Are there any immediate drawbacks from not doing this, except consistency? Also if we decide to do this it needs mention in the developer notes.

practicalswift commented at 11:13 PM on September 6, 2017: contributor

@laanwj In addition to the consistency argument you raised, it could be argued that we should try to avoid deprecated stuff in general :-) The use of the .h form for C compatibility headers is deprecated in C++11. I'll add a note about it in the developer notes.

To make reviewing easy I made this change as a scripted-diff.

practicalswift force-pushed on Sep 6, 2017

fanquake added the label Refactoring on Sep 6, 2017

practicalswift commented at 11:51 PM on September 6, 2017: contributor

Added a note about the preference of #include <cxxx> in the developer notes.

Another argument for normalization is to allow for easy detection of duplicate includes and quick grepping for all usages of say <csignal> (instead of having to search for both <csignal> and <signal.h>).

scripted-diff: Use <cxxx> instead of deprecated <xxx.h> when including C compatibility headers

Includes changed:

```
<assert.h>
<errno.h>
<limits.h>
<math.h>
<signal.h>
<stdarg.h>
<stdint.h>
<stdio.h>
<stdlib.h>
<string.h>
```

-BEGIN VERIFY SCRIPT-
for FILE in \
    src/*.cpp \
    src/*.h \
    src/bench/*.cpp \
    src/bench/*.h \
    src/compat/*.cpp \
    src/compat/*.h \
    src/consensus/*.cpp \
    src/consensus/*.h \
    src/crypto/*.cpp \
    src/crypto/*.h \
    src/primitives/*.cpp \
    src/primitives/*.h \
    src/qt/*.cpp \
    src/qt/*.h \
    src/rpc/*.cpp \
    src/rpc/*.h \
    src/script/*.cpp \
    src/script/*.h \
    src/support/*.cpp \
    src/support/*.h \
    src/test/*.cpp \
    src/test/*.h \
    src/wallet/*.cpp \
    src/wallet/*.h \
    src/zmq/*.cpp \
    src/zmq/*.h \
    ; do
    sed -i 's/#include <\(assert\|errno\|limits\|math\|signal\|stdarg\|stdint\|stdio\|stdlib\|string\).h>/#include <c\1>/g' "$FILE"
done
-END VERIFY SCRIPT-

514e62b330

Document the preference of "#include <cxxx>" over "#include <xxx.h>" when including C compatibility headers 2942efa19a

practicalswift force-pushed on Sep 7, 2017

dcousens approved

danra commented at 10:03 AM on September 11, 2017: contributor

NACK I would love to see this change. However, it would also require using all included functionality through the std namespace, since the non-legacy headers aren't guaranteed to contain it in the global namespace. If you can get the bigger change accepted (prefixing included things with std:: / adding the appropriate using declarations where needed) I would gladly support it.

practicalswift commented at 1:49 PM on September 11, 2017: contributor

@danra using namespace ... is not allowed (see developer-notes.md), and I'm afraid global prefixing would produce a gigantic diff. Just changing {,u}int{8,16,32,64}_t to std::{,u}int{8,16,32,64}_t would touch >2300 lines! :-)

$ git grep -E '[^a-z](u|)int(8|16|32|64)_t[^a-z]' -- "*.cpp" "*.h" | wc -l
2363

danra commented at 3:17 PM on September 11, 2017: contributor

@practicalswift You can use non-namespace using declarations. But regardless, I doubt this kind of big change would be approved. But given that, doing your proposed change would make the code less compliant with the C++ standard, not more so. In fact, to maximize portability and compliance with the standard, there are probably many instances of #include <cxxx> in the project which should be replaced with #include <xxx.h>, which is guaranteed to have its functionality visible in both the std and the global namespaces.

practicalswift commented at 4:19 PM on September 11, 2017: contributor

Closing this PR due to failure to reach ACK consensus :-)

practicalswift closed this on Sep 11, 2017

practicalswift deleted the branch on Apr 10, 2021

DrahtBot locked this on Aug 18, 2022