Add explicit moc includes to sources for moc-covered headers #704
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Montel
commented
Oct 23, 2023
Montel
commented
- speeds up incremental builds as changes to a header will not always need the full mocs_compilation.cpp for all the target's headers rebuild, while having a moc file sourced into a source file only adds minor extra costs, due to small own code and the used headers usually already covered by the source file, being for the same class/struct
- seems to not slow down clean builds, due to empty mocs_compilation.cpp resulting in those quickly processed, while the minor extra cost of the sourced moc files does not outweigh that in summary. Measured times actually improved by some percent points. (ideally CMake would just skip empty mocs_compilation.cpp & its object file one day)
- enables compiler to see all methods of a class in same compilation unit to do some sanity checks
- potentially more inlining in general, due to more in the compilation unit
- allows to keep using more forward declarations in the header, as with the moc code being sourced into the cpp file there definitions can be ensured and often are already for the needs of the normal class methods
* speeds up incremental builds as changes to a header will not always need the full mocs_compilation.cpp for all the target's headers rebuild, while having a moc file sourced into a source file only adds minor extra costs, due to small own code and the used headers usually already covered by the source file, being for the same class/struct * seems to not slow down clean builds, due to empty mocs_compilation.cpp resulting in those quickly processed, while the minor extra cost of the sourced moc files does not outweigh that in summary. Measured times actually improved by some percent points. (ideally CMake would just skip empty mocs_compilation.cpp & its object file one day) * enables compiler to see all methods of a class in same compilation unit to do some sanity checks * potentially more inlining in general, due to more in the compilation unit * allows to keep using more forward declarations in the header, as with the moc code being sourced into the cpp file there definitions can be ensured and often are already for the needs of the normal class methods
| @@ -20,3 +20,5 @@ MyObject::MyObject(QObject* parent) | |||
| , ::rust::cxxqtlib1::CxxQtLocking() | |||
| { | |||
| } | |||
|
|
|||
| #include "moc_inheritance.cpp" | |||
There was a problem hiding this comment.
Note these files are just test outputs for the test inputs. Eg the expected C++ output for the given C++ input. (these ones aren't actually compiled)
Should these have explicit moc includes ? As we would then need to add this to the generation phase which then needs to know the resultant file name.
Or should these be added to the cxx-qt-lib sources instead ?
There was a problem hiding this comment.
ok I didn't know that it was not compiled.
|
If I understand the previous discussion correctly, this can be closed? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.