Header include guideline #10575

1. 
   sipa commented at 10:53 pm on June 11, 2017: member
2. fanquake added the label Docs and Output on Jun 11, 2017
3. Header include guideline a090d1c1c9
4. sipa force-pushed on Jun 11, 2017
5. gmaxwell approved
6. 
   gmaxwell commented at 11:52 pm on June 11, 2017: contributor

   ACK
7. 
   practicalswift commented at 2:25 am on June 12, 2017: contributor

   ACK!
8. 
   fanquake commented at 4:39 am on June 12, 2017: member

   ACK a090d1c
9. 
   paveljanik commented at 5:02 am on June 12, 2017: contributor

   ACK https://github.com/bitcoin/bitcoin/pull/10575/commits/a090d1c1c9c379bce7345ba5b78aa9491e350d0b
10. 
    jonasschnelli commented at 7:16 am on June 12, 2017: contributor

    Good! ACK a090d1c1c9c379bce7345ba5b78aa9491e350d0b
11. 
    laanwj commented at 7:42 am on June 12, 2017: member

    Makes sense.

    Is there a good way to check/review this, though? Would be useful to add suggestion for reviewing.

    I’m also afraid this will generate lots of ‘mess around with headers’ PRs, so we probably want to add that this holds for new code.

12. 
    practicalswift commented at 9:01 am on June 12, 2017: contributor

    @laanwj I wrote the following script …

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

    … to identify .cpp files that re-include the includes already included in its corresponding .h file. See PR #10574.

13. 
    laanwj commented at 9:53 am on June 12, 2017: member

    Well yes but I’m more interested in the first point:

    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.

    So how to check that a .cpp or .h is using functionality that is only indirectly included? Seems like a lot of work to do manually, at least.

14. 
    practicalswift commented at 10:24 am on June 12, 2017: contributor

    @laanwj Ah, iwyu is probably best-in-class there!
15. 
    ryanofsky commented at 1:03 pm on June 12, 2017: member

    Iwyu is great and it would be really nice if it works on bitcoin source. It manages forward declarations as well as includes and will automatically add/remove/convert between them as appropriate.
16. 
    sipa commented at 6:47 pm on June 12, 2017: member

    Perhaps we should have iwyu included in our build system, and then refer to it in developer-notes.md?
17. 
    laanwj commented at 7:19 am on June 13, 2017: member

    Iwyu is great and it would be really nice if it works on bitcoin source

    Has anyone tried?

    Edit: anyhow, that discussion does not have to block this. If you get that tool to work, it would be appreciated if you file a PR how to use it with our source.

18. laanwj merged this on Jun 13, 2017
19. laanwj closed this on Jun 13, 2017

    ---

20. laanwj referenced this in commit 22ec768838 on Jun 13, 2017
21. sipa deleted the branch on Jun 23, 2017
22. laanwj referenced this in commit 5d132e8b97 on Dec 12, 2017
23. practicalswift referenced this in commit cf50ec1dfa on Dec 12, 2017
24. practicalswift referenced this in commit c614e4ce4d on Dec 13, 2017
25. practicalswift referenced this in commit b1b9c4345e on Dec 13, 2017
26. practicalswift referenced this in commit 4dbfc1a83f on Dec 13, 2017
27. practicalswift referenced this in commit b1ef6bdcd4 on Mar 14, 2018
28. practicalswift referenced this in commit eb8ba7af25 on Mar 19, 2018
29. practicalswift referenced this in commit 745513ecb7 on Mar 19, 2018
30. practicalswift referenced this in commit c36b720d00 on Apr 9, 2018
31. MarcoFalke referenced this in commit a04440feb9 on Apr 9, 2018
32. stamhe referenced this in commit 76e9046eec on Jun 27, 2018
33. HashUnlimited referenced this in commit 0128379184 on Aug 15, 2018
34. lionello referenced this in commit b0a545a394 on Nov 7, 2018
35. joemphilips referenced this in commit dc40370383 on Nov 9, 2018
36. PastaPastaPasta referenced this in commit 4976658734 on Jul 5, 2019
37. PastaPastaPasta referenced this in commit 543f32c238 on Jul 5, 2019
38. PastaPastaPasta referenced this in commit 1fd3160824 on Jul 6, 2019
39. PastaPastaPasta referenced this in commit 5ec275ffe5 on Jul 8, 2019
40. PastaPastaPasta referenced this in commit e4fdd13a0e on Jul 9, 2019
41. PastaPastaPasta referenced this in commit 10716ea6dd on Jul 9, 2019
42. barrystyle referenced this in commit 60e1c15164 on Jan 22, 2020
43. PastaPastaPasta referenced this in commit fd65b9589f on Mar 27, 2020
44. PastaPastaPasta referenced this in commit 5a46d18116 on Mar 28, 2020
45. PastaPastaPasta referenced this in commit 977a080fd9 on Mar 29, 2020
46. PastaPastaPasta referenced this in commit b93fc710d5 on Mar 31, 2020
47. UdjinM6 referenced this in commit f6e1b28661 on Mar 31, 2020
48. PastaPastaPasta referenced this in commit fe8c18ca77 on Apr 1, 2020
49. PastaPastaPasta referenced this in commit 949d1685e3 on Jun 17, 2020
50. PastaPastaPasta referenced this in commit 005d241fea on Jul 2, 2020
51. ckti referenced this in commit bffacf8cc5 on Mar 28, 2021
52. DrahtBot locked this on Sep 8, 2021

Contributors
sipa gmaxwell practicalswift fanquake paveljanik jonasschnelli laanwj ryanofsky

Labels
Docs