jonathonl
commented
2 years ago To be honest, I'm not that worried about this. It's perfectly fine to not explicitly include every header needed for a file. For example, If a file includes <sstream>, then <iostream> will always be included because stringstream derives from istream and ostream. Excessively redundant includes could theoretically slow down compilation, and while this could help future proof against changes in the stdlib, there are plenty of other ways that a future compiler version could break a build (e.g., stricter adherence to c++ standard). When I get a chance, I'll review this text file and clean up where needed, but this is not a high priority.
As noted in https://github.com/statgen/savvy/issues/26 and discovered in other dependent projects, Savvy has some symbols used that are not specifically included in files. This is going to continue to cause portability problems between different systems and C compiler / library versions in the future unless fixed.
One potential way to tackle this is to use a tool like https://github.com/include-what-you-use/include-what-you-use to audit the code and find missing imports. It's available on Debian, Ubuntu, and Homebrew package managers too. I did a very quick run of it (without mapping properly set up on my system), and got these results, which are a pretty good starting point:
savvy_iwyu_results.txt
I'd be open to working through these as a checklist, but integrating a tool like iwyu into the cmake build process could potentially automatically fix problems like these both now and in the future.