Remove includes in .cpp files for things the corresponding .h file already included #10574

practicalswift · 2017-06-11T21:41:01Z

Remove includes in .cpp files for things the corresponding .h file already included.

Example case:

addrdb.cpp includes addrdb.h and fs.h
addrdb.h includes fs.h

Then remove the direct inclusion of fs.h in addrman.cpp and rely on the indirect inclusion of fs.h via the included addrdb.h.

In line with the header include guideline (see #10575).

practicalswift · 2017-06-12T08:18:56Z

Candidates for removal found by:

#!/bin/bash

for H in $(git grep "" -- "*.h" | cut -f1 -d: | uniq); do
    CPP=${H/%\.h/.cpp}
    if [[ ! -e $CPP ]]; then
        continue
    fi
    cat "$H" "$CPP" | grep -E "^#include " | sort | uniq -c | \
      grep " 2 #include" && echo "$CPP" && echo
done

practicalswift · 2017-08-09T08:42:52Z

Anyone willing to review? :-)

razamobin · 2017-08-10T22:08:29Z

Hi, I'm new to the bitcoin source code, but I looked for redundant includes and got the same changeset as you. Also I was able to run unit tests and they all passed (although I guess Travis did this already?). Either way, looks good to me.

luke-jr · 2017-09-02T04:03:22Z

Soft NACK. IMO it's better code quality to explicitly include everything needed in each source file using them.

practicalswift · 2017-09-10T19:08:26Z

Rebased! :-)

danra · 2017-09-10T19:50:51Z

I agree with luke-jr

sipa · 2017-09-10T19:53:55Z

@luke-jr @danra Then you should argue to change the guidelines in developer-notes.md:

Every .cpp and .h file should #include every header file it directly uses classes, functions or other definitions from, even if those headers are already included indirectly through other headers. One exception is that a .cpp file does not need to re-include the includes already included in its corresponding .h file.

I personally think that rule makes sense, as you can see the .cpp file as the continuation of the corresponding .h file.

danra · 2017-09-10T20:12:41Z

@sipa Yes, I'm for changing this rule as it creates an unnecessary dependency of the implementation (.cpp) on the interface (.h). If the interface no longer uses some other include directly, but the implementation still uses that include directly, removing the include from the interface would require adding it to the implementation.

(Note I'm not for having the implementation re-include every include of the interface - only those which it uses directly)

sipa · 2017-09-10T22:39:09Z

@danra I understand that point of view, but I think it's a non-issue. When the interface changes, almost certainly the implementation needs to change anyway.

practicalswift · 2017-09-29T12:51:48Z

Rebased! :-)

…ready included

practicalswift · 2017-11-16T21:28:40Z

Rebased! Getting this merged would get rid of 76 lines of redundant includes :-)

…ing .h file already included a720b92 Remove includes in .cpp files for things the corresponding .h file already included (practicalswift) Pull request description: Remove includes in .cpp files for things the corresponding .h file already included. Example case: * `addrdb.cpp` includes `addrdb.h` and `fs.h` * `addrdb.h` includes `fs.h` Then remove the direct inclusion of `fs.h` in `addrman.cpp` and rely on the indirect inclusion of `fs.h` via the included `addrdb.h`. In line with the header include guideline (see #10575). Tree-SHA512: 8704b9de3011a4c234db336a39f7d2c139e741cf0f7aef08a5d3e05197e1e18286b863fdab25ae9638af4ff86b3d52e5cab9eed66bfa2476063aa5c79f9b0346

laanwj · 2017-12-12T14:01:57Z

utACK a720b92

promag · 2017-12-12T14:06:48Z

src/qt/platformstyle.cpp

 #include <QImage>
 #include <QPalette>
-#include <QPixmap>


This is in the header BUT I believe it's not necessary there. IMO the first exercise should be to reduce to a minimal the includes in a header file.

…responding .h file already included a720b92 Remove includes in .cpp files for things the corresponding .h file already included (practicalswift) Pull request description: Remove includes in .cpp files for things the corresponding .h file already included. Example case: * `addrdb.cpp` includes `addrdb.h` and `fs.h` * `addrdb.h` includes `fs.h` Then remove the direct inclusion of `fs.h` in `addrman.cpp` and rely on the indirect inclusion of `fs.h` via the included `addrdb.h`. In line with the header include guideline (see bitcoin#10575). Tree-SHA512: 8704b9de3011a4c234db336a39f7d2c139e741cf0f7aef08a5d3e05197e1e18286b863fdab25ae9638af4ff86b3d52e5cab9eed66bfa2476063aa5c79f9b0346

…onding .h file already included Summary: Remove includes in .cpp files for things the corresponding .h file already included. Example case: addrdb.cpp includes addrdb.h and fs.h addrdb.h includes fs.h Then remove the direct inclusion of fs.h in addrman.cpp and rely on the indirect inclusion of fs.h via the included addrdb.h. In line with the header include guideline (see #10575). Candidates for removal found by: ```#!/bin/bash for H in $(git grep "" -- "*.h" | cut -f1 -d: | uniq); do CPP=${H/%\.h/.cpp} if [[ ! -e $CPP ]]; then continue fi cat "$H" "$CPP" | grep -E "^#include " | sort | uniq -c | \ grep " 2 #include" && echo "$CPP" && echo done``` --- Backport of Core [[bitcoin/bitcoin#10574 | PR10574]] Test Plan: ninja check-all Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6666

7a5d181 Use the character based overload for std::string::find. (Alin Rus) c19401b Don't use pass by reference to const for cheaply-copied types (bool, char, etc.). (practicalswift) 4c5fe36 [Refactor] Remove unused fQuit var from checkqueue.h (donaloconnor) fda7a5f Cleanup: remove unneeded header includes (random-zebra) ac2476c Add test cases covering the relevant key length boundaries: 64 bytes +/- 1 byte for HMAC-SHA256 and 128 bytes +/- 1 byte for HMAC-SHA512 (practicalswift) a346262 Fix code constness in CBlockIndex::GetAncestor() overloads (Dan Raviv) 65e3f4e Refactor: More range-based for loops (random-zebra) dd3d3c4 Use range-based for loops (C++11) when looping over map elements (practicalswift) 5a7750a Move RPC registration out of AppInitParameterInteraction (Russell Yanofsky) 402b4c4 Use compile-time constants instead of unnamed enumerations (practicalswift) bbac2d0 [Trivial] Fix indentation in coins.cpp (random-zebra) e539c62 Small refactor of CCoinsViewCache::BatchWrite() (Dan Raviv) ec91759 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (random-zebra) 93487b1 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (random-zebra) ff43d69 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift) b4d9641 Use unique_ptr for dbenv (DbEnv) (practicalswift) 36108b9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift) ff1c454 Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift) 93daf17 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift) 020ac16 Init: Remove redundant exit(EXIT_FAILURE) instances and replace with (random-zebra) b9f5d1f Remove duplicate uriParts.size() > 0 check (practicalswift) 440d961 Remove redundant check (!ecc is always true) (practicalswift) bfd295b Remove redundant NULL checks after new (practicalswift) 97aad32 Make fUseCrypto atomic (MeshCollider) 2711f78 Consistent parameter names in txdb.h (MeshCollider) d40df3a Fix race for mapBlockIndex in AppInitMain (random-zebra) 03b7766 Cleanup: remove unused functions to Hash the concat of 4 or more objects (random-zebra) c520e0f Remove some unused functions and methods (Pieter Wuille) 508f1a1 range-based loops and const qualifications in net.cpp (Marko Bencun) 79b1e50 Refactor: implement CPubKey::data() (random-zebra) 614d26c Refactor: more &vec[0] to vec.data() (random-zebra) 02b6337 Changing &vec[0] to vec.data(), what 9804 missed (MeshCollider) c1c8b05 Ensure that data types are consistent (jjz) 732bb9d Fix potential null dereferences (MeshCollider) 80f35f9 Remove unreachable code (practicalswift) Pull request description: This is a collection of simple refactorings coming from upstream Bitcoin v0.16 (adapting/extending to PIVX-specific code where needed). Pull requests backported: - bitcoin#10845 (practicalswift) - bitcoin#11238 (MeshCollider) - bitcoin#11232 (jjz) - bitcoin#10793 (MeshCollider) - bitcoin#10888 (benma) - ~~bitcoin#11351 (danra)~~ [edit: removed - included in #2423] - bitcoin#11385 (sipa) - bitcoin#11107 (MeshCollider) - bitcoin#10898 (practicalswift) - bitcoin#11511 (donaloconnor) - bitcoin#11043 (practicalswift) - bitcoin#11353 (danra) - bitcoin#10749 (practicalswift) - bitcoin#11603 (ryanofsky) - bitcoin#10493 (practicalswift) - bitcoin#11337 (danra) - bitcoin#11516 (practicalswift) - bitcoin#10574 (practicalswift) - bitcoin#12108 (donaloconnor) - bitcoin#10839 (practicalswift) - bitcoin#12159 (kekimusmaximus) ACKs for top commit: Fuzzbawls: ACK 7a5d181 furszy: re-ACK 7a5d181 after rebase, no code changes. Merging.. Tree-SHA512: d92f5df47f443391a6531274a2efb9a4882c62d32eff628f795b3abce703f108c8b40ec80ac841cde6c5fdd5c9ff2b6056a31546ac2edda279f5f18fccc99c32

…responding .h file already included a720b92 Remove includes in .cpp files for things the corresponding .h file already included (practicalswift) Pull request description: Remove includes in .cpp files for things the corresponding .h file already included. Example case: * `addrdb.cpp` includes `addrdb.h` and `fs.h` * `addrdb.h` includes `fs.h` Then remove the direct inclusion of `fs.h` in `addrman.cpp` and rely on the indirect inclusion of `fs.h` via the included `addrdb.h`. In line with the header include guideline (see bitcoin#10575). Tree-SHA512: 8704b9de3011a4c234db336a39f7d2c139e741cf0f7aef08a5d3e05197e1e18286b863fdab25ae9638af4ff86b3d52e5cab9eed66bfa2476063aa5c79f9b0346

fanquake added the Refactoring label Jun 11, 2017

practicalswift mentioned this pull request Jun 12, 2017

Header include guideline #10575

Merged

practicalswift force-pushed the redundant branch 3 times, most recently from c364462 to e987b40 Compare June 12, 2017 18:36

practicalswift force-pushed the redundant branch from e987b40 to 269f51b Compare July 2, 2017 13:55

practicalswift force-pushed the redundant branch 2 times, most recently from efd4a9f to 798478e Compare July 24, 2017 14:27

practicalswift force-pushed the redundant branch from 798478e to b256c4b Compare August 9, 2017 08:42

practicalswift force-pushed the redundant branch from b256c4b to 6ab24ac Compare September 10, 2017 19:07

practicalswift force-pushed the redundant branch from 6ab24ac to dc093cc Compare September 29, 2017 12:51

Remove includes in .cpp files for things the corresponding .h file al…

a720b92

…ready included

practicalswift force-pushed the redundant branch from dc093cc to a720b92 Compare November 16, 2017 21:27

laanwj merged commit a720b92 into bitcoin:master Dec 12, 2017

promag reviewed Dec 12, 2017

View reviewed changes

maflcko mentioned this pull request Dec 13, 2017

Remove unused include in hash.cpp #11884

Merged

practicalswift deleted the redundant branch April 10, 2021 19:33

random-zebra mentioned this pull request Jun 29, 2021

[Refactoring] Bitcoin 0.16 cherries PIVX-Project/PIVX#2462

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove includes in .cpp files for things the corresponding .h file already included #10574

Remove includes in .cpp files for things the corresponding .h file already included #10574

Uh oh!

practicalswift commented Jun 11, 2017 •

edited

Loading

Uh oh!

practicalswift commented Jun 12, 2017 •

edited

Loading

Uh oh!

practicalswift commented Aug 9, 2017

Uh oh!

razamobin commented Aug 10, 2017

Uh oh!

luke-jr commented Sep 2, 2017

Uh oh!

practicalswift commented Sep 10, 2017

Uh oh!

danra commented Sep 10, 2017

Uh oh!

sipa commented Sep 10, 2017 •

edited

Loading

Uh oh!

danra commented Sep 10, 2017

Uh oh!

sipa commented Sep 10, 2017

Uh oh!

practicalswift commented Sep 29, 2017

Uh oh!

practicalswift commented Nov 16, 2017 •

edited

Loading

Uh oh!

laanwj commented Dec 12, 2017

Uh oh!

promag Dec 12, 2017

Uh oh!

Uh oh!

Remove includes in .cpp files for things the corresponding .h file already included #10574

Remove includes in .cpp files for things the corresponding .h file already included #10574

Uh oh!

Conversation

practicalswift commented Jun 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Jun 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Aug 9, 2017

Uh oh!

razamobin commented Aug 10, 2017

Uh oh!

luke-jr commented Sep 2, 2017

Uh oh!

practicalswift commented Sep 10, 2017

Uh oh!

danra commented Sep 10, 2017

Uh oh!

sipa commented Sep 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danra commented Sep 10, 2017

Uh oh!

sipa commented Sep 10, 2017

Uh oh!

practicalswift commented Sep 29, 2017

Uh oh!

practicalswift commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Dec 12, 2017

Uh oh!

promag Dec 12, 2017

Choose a reason for hiding this comment

Uh oh!

Uh oh!

practicalswift commented Jun 11, 2017 •

edited

Loading

practicalswift commented Jun 12, 2017 •

edited

Loading

sipa commented Sep 10, 2017 •

edited

Loading

practicalswift commented Nov 16, 2017 •

edited

Loading