Don't remove direct Qt's and std's includes
Hi, I’m working on the engine for displaying, playing and manipulating the score in the official MoseScore mobile application, and development a new feature for the editor.
Therefore, I work with the MoseScore code everyday, and I have an important issue for me.
For development, I use the QtCreator, it’s a very cool and powerful IDE.
To help develop, the QtCreator build a code model, and due to this model they work correctly:
- code highlighting
- auto completion
- code jump
- code using
- rename
- and other
In order for a QtCreator to build a code model, he needs to have information about the types used, which is provided using inclusions of these types (including standard Qt and std).
There is an IDE, for example VS Code, which work due to parsing syntax and search, so they don’t need a code model, they don’t need icludes, but they do not allow the correct implementation of a number of possibilities (jump, rename, etc.)
MoseScore uses a header precompiler to speed up build, which is good.
But it is customary to remove direct includes Qt and std (for example https://github.com/musescore/MuseScore/pull/5704, https://github.com/musescore/MuseScore/pull/5692) , and therefore the QtCreator cannot build a code model, and this is a problem for me.
Direct inclusion of types used in the file does not interfere with working with precompiled headers and does not affect the build speed. They are not inserted by the preprocessor because they are actually already inserted by precompiled header (the insert is protected by a guard). Аt the same time, gives a lot of benefits, which may or may not be used.
Also, the direct inclusion of headers gives an understanding of the organization of the code structure. For example, if in a class like Chord, if there is something like QNetworkAccessManager, or QMessageBox, etc., it means something went wrong.
Without the direct inclusion of headers, it is more difficult to see the incorrect structure of the code, because you cannot see what the class really depends on, especially if it is very large (2000-3000 and more lines of code).
Can we change the approach, and don't remove the direct inclusion of the headers Qt and std?
Comments
Or move those all into this all.h and use
#include "all.h"in all the source files (needing any of them) rather than using that command line option to included at build time?Not sure how that influences compile time and PCH though.
OTOH I also do like the idea for every source file (either .cpp or the .h with the same basename included in the former) to
#includeeverything it directly needs and uses and that way makes every source file 'standalone', self-contained, self-documenting its pre-requisits or whatever you want to name it.Currently also discussed in the MuseScore developers' chat on Telegram, maybe you'd want to join there?
I realise I'm a bit late to this party, but I've finally found some time to consider the pros and cons around PCH/GCH usage. So here are my views on file inclusion.
Disclaimer: I have only dug into MSVC/gcc pre compiled header behavior
What should be included in the PCH/GCH
"Common" header files that are not owned by us and due to the project nature hardly ever change.
What should be included in our own header files
First off: each header files must have written out a file guard against multiple inclusion. Don't use "
#pragma once" as pragma directives by nature are toolchain specific.Write your includes as if PCH does not exist. You should include only the header files that are strictly necessary to correctly interpret the header file as a standalone compilation unit. Do not include headers from classes where the interface definition is irrelevant, use forward declaration instead.
Your include statements should not assume any knowledge about includes made by the files you include. If your file needs "a.h" and "b.h" it should include both of them, even when "a.h" includes "b.h" itself.
What should be included in our own source files
If your source file is an interface/class implementation file, it must include the interface definition header file and preferrably do so as the first include.
Then, just as with the header files above, write the includes as if PCH is not a thing and so that the file stands on its own as a compilation unit. Do not
#include "all.h".For implementation source files, there is no need to re-include the files included by the interface definition header.
Motivation
PCH is a toolchain feature, and not a language feature. Therefor PCH inclusion should be handled by the toolchain. Manual explicit inclusion of the PCH into source files will only result in longer parsing and compilation times for those that build without PCH (as the PCH includes more than what is strictly required for that translation unit) and not have any additional gains for those that build with PCH enabled.
Many IDEs will be able to offer code completion and their parsers will only have to deal with the files strictly required for this translation unit. The penalty for checking the include guard for each included file is negligible compared to compilation time.
By not explicitly including all.h those IDE parsers have also less content to parse for a translation unit, so the IDE experience should involve less waiting on auto-completion after loading in a source file.
Side note
I personally have the habit of listing includes in the following order:
Usually there is a single empty line separating these groups and the includes within them are sorted alphabetically (which helps to quickly locate them and prevent duplicates)