Improve includes #6320
Improve includes #6320
Conversation
E.g.: assert.h -> cassert
This allows to remove includes to TrackView.h in ClipView cpp files.
🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩
Linux
Windows
macOS🤖{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://15834-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.182%2Bg7d1c982ab-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15834?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://15833-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.182%2Bg7d1c982ab-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15833?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://15836-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.182%2Bg7d1c982ab-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15836?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/citejhkk2uxx1rh2/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/42749613"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/o1nrj7qqo0r9itws/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/42749613"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://15835-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.182%2Bg7d1c982ab-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15835?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "ddcdc256a1b0d675e355a0f1ddfd5d1b0b6c589b"} |
This improves the include structure by elliminating includes that are
not used. Most of this was done by using `include-what-you-use` with
`CMAKE_C_INCLUDE_WHAT_YOU_USE` and `CMAKE_CXX_INCLUDE_WHAT_YOU_USE`
set to (broken down here):
```
include-what-you-use;
-Xiwyu;--mapping_file=/usr/share/include-what-you-use/qt5_11.imp;
-Xiwyu;--keep=*/xmmintrin.h;
-Xiwyu;--keep=*/lmmsconfig.h;
-Xiwyu;--keep=*/weak_libjack.h;
-Xiwyu;--keep=*/sys/*;
-Xiwyu;--keep=*/debug.h;
-Xiwyu;--keep=*/SDL/*;
-Xiwyu;--keep=*/alsa/*;
-Xiwyu;--keep=*/FL/x.h;
-Xiwyu;--keep=*/MidiApple.h;
-Xiwyu;--keep=*/MidiWinMM.h;
-Xiwyu;--keep=*/AudioSoundIo.h
```
b1208d3 to
068a0f8
Compare
|
I reworked your comments and pushed. For some headers, I think this PR can damage more than it should. For example, if you remove "lmmsconfig.h" accidentally, the code might still compile, but be configured wrong. Though, I didn't notice more headers with that issue, and at least for this one, the worst case seems to be "LMMS compiles more than I wanted". Another point (already mentioned in one comment): The include order in this PR is not too crucial, as clang-format will reorder all of this - except if there are |
There was a problem hiding this comment.
For some headers, I think this PR can damage more than it should. For example, if you remove "lmmsconfig.h" accidentally, the code might still compile, but be configured wrong. Though, I didn't notice more headers with that issue, and at least for this one, the worst case seems to be "LMMS compiles more than I wanted".
I'm not sure if there's an easy way to check for this. CI is passing, which is the main thing, so I'd say we merge this and fix up any issues later.
|
I've also checked with more compilers, and it's working. 👍 |
|
@JohannesLorenz I'm on a brand new laptop now and Ubuntu 21.10, fresh install. Building CXX object src/CMakeFiles/lmmsobjs.dir/core/MixHelpers.cpp.o
/home/zonkmachine/builds/lmms/src/core/MixHelpers.cpp: In function ‘bool MixHelpers::sanitize(sampleFrame*, int)’:
/home/zonkmachine/builds/lmms/src/core/MixHelpers.cpp:105:41: error: ‘printf’ was not declared in this scope
105 | printf("Bad data, clearing buffer. frame: ");
| ^~~~~~
/home/zonkmachine/builds/lmms/src/core/MixHelpers.cpp:32:1: note: ‘printf’ is defined in header ‘<cstdio>’; did you forget to ‘#include <cstdio>’?
31 | #include "ValueBuffer.h"
+++ |+#include <cstdio>
32 |
make[2]: *** [src/CMakeFiles/lmmsobjs.dir/build.make:652: src/CMakeFiles/lmmsobjs.dir/core/MixHelpers.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:2319: src/CMakeFiles/lmmsobjs.dir/all] Error 2
make: *** [Makefile:171: all] Error 2 |
|
It seems like debug builds(with |
|
Confirmed that it's the only build error, at least on Linux with GCC 11 and Qt 5.15. |
|
When added back, that include ought to be wrapped in |
As requested in #5592 .
The last commit has been done using iwyu (see commit for details) and a lot of manual refactoring. The commits before it are preparation.