Fix PCH detection in depend mode (+test improvements) #427
Conversation
This file contains hidden or 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
The PTH feature has been removed (https://reviews.llvm.org/D54547) and according to the commit it has never really been used. Maybe this made sense somewhen in the past, but now those .pth files must be PCHs internally. This commit actually just changes the .pth extensions to .pch to avoid confusion, technically nothing should change except for filenames.
Clang is supposed to be a drop-in for GCC, so in general it should be able to handle everything GCC can. That's not completely true in practice, there are differences, but it doesn't make sense to completely duplicate a testcase just because there are some differences. So start creating a shared common base for the PCH tests and do separately only tests that act differently.
create pch -> pch_defines use pch -> time_macros
There's e.g. no need to do all kinds of complex tests with both .gch and .pch with Clang, except for checking that Clang finds one of them if none is specified explicitly.
Some of the tests did that, e.g. those 'file changed', but e.g. the cached .gch creation did not. So try to intentionally change the .gch/.h and test that it leads to a cache miss. Otherwise there might be a hit simply because ccache failed to detect PCH usage and ignores the .gch completely.
As the manpage says, -fpch-preprocess is needed only with the #include form, otherwise it's pointless.
Since sloppiness is normally required, so no point in stating the obvious.
GCC does not output the pch in the .d dependencies file, so without this there would be false cache hits.
- ccache will fail to properly detect that -include a.h means using a.h.gch if it requires using path from -I (they are not searched) - -fpch-preprocess does nothing with Clang, it doesn't output pragma GCC pch_preprocess and so #include form for PCHs doesn't work
|
Excellent, thanks! |
|
Thank you for merging. I'd like to add a configure check for the bug, meaning the ccache version that doesn't have it. Is that going to be 3.8? |
Yes, unless some more severe bug pops up which motivates a 3.7.2 release – in that case your bug fix will be included there as well. |
jrosdahl
pushed a commit
to jrosdahl/ccache
that referenced
this pull request
Jul 18, 2019
* do not refer to Clang's PTH in tests The PTH feature has been removed (https://reviews.llvm.org/D54547) and according to the commit it has never really been used. Maybe this made sense somewhen in the past, but now those .pth files must be PCHs internally. This commit actually just changes the .pth extensions to .pch to avoid confusion, technically nothing should change except for filenames. * try to share PCH tests between GCC and Clang Clang is supposed to be a drop-in for GCC, so in general it should be able to handle everything GCC can. That's not completely true in practice, there are differences, but it doesn't make sense to completely duplicate a testcase just because there are some differences. So start creating a shared common base for the PCH tests and do separately only tests that act differently. * more sharing of PCH tests between GCC and Clang There's e.g. no need to do all kinds of complex tests with both .gch and .pch with Clang, except for checking that Clang finds one of them if none is specified explicitly. * log also when pch usage is detect from pragma pch_preprocess * try harder to verify in tests that ccache detects PCH changes Some of the tests did that, e.g. those 'file changed', but e.g. the cached .gch creation did not. So try to intentionally change the .gch/.h and test that it leads to a cache miss. Otherwise there might be a hit simply because ccache failed to detect PCH usage and ignores the .gch completely. * clean up #include vs -fpch-preprocess in pch tests As the manpage says, -fpch-preprocess is needed only with the #include form, otherwise it's pointless. * do not mention sloppiness in pch tests, only no sloppiness Since sloppiness is normally required, so no point in stating the obvious. * test also -include-pch with clang * hash also pch introduced only using -include GCC does not output the pch in the .d dependencies file, so without this there would be false cache hits. * be consistent about sloppiness in pch tests create pch -> pch_defines use pch -> time_macros * test CCACHE_PCH_EXTSUM more thoroughly and also with -include * pch test for .gch file being in an extra directory * doc corrections for how to use PCH with ccache - ccache will fail to properly detect that -include a.h means using a.h.gch if it requires using path from -I (they are not searched) - -fpch-preprocess does nothing with Clang, it doesn't output pragma GCC pch_preprocess and so #include form for PCHs doesn't work * explain better problems of -MD/-MMD in depend mode
jrosdahl
pushed a commit
to jrosdahl/ccache
that referenced
this pull request
Jul 19, 2019
* do not refer to Clang's PTH in tests The PTH feature has been removed (https://reviews.llvm.org/D54547) and according to the commit it has never really been used. Maybe this made sense somewhen in the past, but now those .pth files must be PCHs internally. This commit actually just changes the .pth extensions to .pch to avoid confusion, technically nothing should change except for filenames. * try to share PCH tests between GCC and Clang Clang is supposed to be a drop-in for GCC, so in general it should be able to handle everything GCC can. That's not completely true in practice, there are differences, but it doesn't make sense to completely duplicate a testcase just because there are some differences. So start creating a shared common base for the PCH tests and do separately only tests that act differently. * more sharing of PCH tests between GCC and Clang There's e.g. no need to do all kinds of complex tests with both .gch and .pch with Clang, except for checking that Clang finds one of them if none is specified explicitly. * log also when pch usage is detect from pragma pch_preprocess * try harder to verify in tests that ccache detects PCH changes Some of the tests did that, e.g. those 'file changed', but e.g. the cached .gch creation did not. So try to intentionally change the .gch/.h and test that it leads to a cache miss. Otherwise there might be a hit simply because ccache failed to detect PCH usage and ignores the .gch completely. * clean up #include vs -fpch-preprocess in pch tests As the manpage says, -fpch-preprocess is needed only with the #include form, otherwise it's pointless. * do not mention sloppiness in pch tests, only no sloppiness Since sloppiness is normally required, so no point in stating the obvious. * test also -include-pch with clang * hash also pch introduced only using -include GCC does not output the pch in the .d dependencies file, so without this there would be false cache hits. * be consistent about sloppiness in pch tests create pch -> pch_defines use pch -> time_macros * test CCACHE_PCH_EXTSUM more thoroughly and also with -include * pch test for .gch file being in an extra directory * doc corrections for how to use PCH with ccache - ccache will fail to properly detect that -include a.h means using a.h.gch if it requires using path from -I (they are not searched) - -fpch-preprocess does nothing with Clang, it doesn't output pragma GCC pch_preprocess and so #include form for PCHs doesn't work * explain better problems of -MD/-MMD in depend mode
llunak
added a commit
to llunak/ccache
that referenced
this pull request
Nov 2, 2019
* do not refer to Clang's PTH in tests The PTH feature has been removed (https://reviews.llvm.org/D54547) and according to the commit it has never really been used. Maybe this made sense somewhen in the past, but now those .pth files must be PCHs internally. This commit actually just changes the .pth extensions to .pch to avoid confusion, technically nothing should change except for filenames. * try to share PCH tests between GCC and Clang Clang is supposed to be a drop-in for GCC, so in general it should be able to handle everything GCC can. That's not completely true in practice, there are differences, but it doesn't make sense to completely duplicate a testcase just because there are some differences. So start creating a shared common base for the PCH tests and do separately only tests that act differently. * more sharing of PCH tests between GCC and Clang There's e.g. no need to do all kinds of complex tests with both .gch and .pch with Clang, except for checking that Clang finds one of them if none is specified explicitly. * log also when pch usage is detect from pragma pch_preprocess * try harder to verify in tests that ccache detects PCH changes Some of the tests did that, e.g. those 'file changed', but e.g. the cached .gch creation did not. So try to intentionally change the .gch/.h and test that it leads to a cache miss. Otherwise there might be a hit simply because ccache failed to detect PCH usage and ignores the .gch completely. * clean up #include vs -fpch-preprocess in pch tests As the manpage says, -fpch-preprocess is needed only with the #include form, otherwise it's pointless. * do not mention sloppiness in pch tests, only no sloppiness Since sloppiness is normally required, so no point in stating the obvious. * test also -include-pch with clang * hash also pch introduced only using -include GCC does not output the pch in the .d dependencies file, so without this there would be false cache hits. * be consistent about sloppiness in pch tests create pch -> pch_defines use pch -> time_macros * test CCACHE_PCH_EXTSUM more thoroughly and also with -include * pch test for .gch file being in an extra directory * doc corrections for how to use PCH with ccache - ccache will fail to properly detect that -include a.h means using a.h.gch if it requires using path from -I (they are not searched) - -fpch-preprocess does nothing with Clang, it doesn't output pragma GCC pch_preprocess and so #include form for PCHs doesn't work * explain better problems of -MD/-MMD in depend mode
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.
(I can split this if you dislike so many commits, but it mostly belongs together. I was looking for the PCH bug and had to extend the tests quite a bit before I finally found it.)
This fixes ccache failing to hash pch file in depend mode if that pch file is introduced only using -include. The rest is adding more PCH tests and removing the needless duplication between GCC and Clang.