Uh oh!
There was an error while loading. Please reload this page.
New makefile to fix dependency errors - #5546
Conversation
chrchr-github
commented
Oct 12, 2023
Seems like dmake.cpp needs to be adapted. |
| $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkfunctions.cpp | ||
| $(libcppdir)/checkinternal.o: lib/checkinternal.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkinternal.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h | ||
| $(libcppdir)/checkinternal.o: lib/checkinternal.cpp |
There was a problem hiding this comment.
in my humble opinion this is wrong. the checkinternal.cpp includes various headers. The CHECK_INTERNAL is sometimes defined.
There was a problem hiding this comment.
Looks like it was not compiled with CHECK_INTERNAL defined.
| $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/cppcheck.cpp | ||
| $(libcppdir)/ctu.o: lib/ctu.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h | ||
| $(libcppdir)/ctu.o: lib/ctu.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/astutils.h lib/check.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h externals/tinyxml2/tinyxml2.h |
There was a problem hiding this comment.
The externals/tinyxml2/tinyxml2.h is already a dependency.
firewave
commented
Oct 12, 2023
Thanks for your contribution. Please provide more details on how you determined this and what issues this might fix. FYI the |
Meiye-lj
commented
Oct 13, 2023
We recently conducted a study to detect build dependency errors, focusing on missing and redundant dependencies. |
| $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checktype.cpp | ||
| $(libcppdir)/checkuninitvar.o: lib/checkuninitvar.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h | ||
| $(libcppdir)/checkuninitvar.o: lib/checkuninitvar.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h externals/tinyxml2/tinyxml2.h |
There was a problem hiding this comment.
If this include was required the code would not build as there is no .h files which includes tinyxml2.h so each .cpp files which needs that information needs to include this.
In the code in question the include is not necessary as the type is forward declared and it used as pointer-only.
There was a problem hiding this comment.
👍 @Meiye-lj please handle forward declarations in your tool because it's an important way to reduce dependencies.
| $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testother.cpp | ||
| test/testpath.o: test/testpath.cpp lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h | ||
| test/testpath.o: test/testpath.cpp lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h externals/simplecpp/simplecpp.h |
There was a problem hiding this comment.
This also appears to be unnecessary. simplecpp.h is only included by preprocessor.h which is not includes by any other .h file. It is used pointer-only so forward declaration will suffice.
| $(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp | ||
| cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/cppcheckexecutorsig.h cli/executor.h cli/filelister.h cli/processexecutor.h cli/singleexecutor.h cli/threadexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h | ||
| cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/cppcheckexecutorsig.h cli/executor.h cli/filelister.h cli/processexecutor.h cli/singleexecutor.h cli/threadexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h externals/simplecpp/simplecpp.h |
There was a problem hiding this comment.
This also appears to be unnecessary. simplecpp.h is only included by preprocessor.h which is not includes by any other .h file. It is used pointer-only so forward declaration will suffice.
firewave
commented
Oct 13, 2023
We are using our own tool We are also Based on your findings it seems there might have been some shortcomings in your study which need to be addressed. |
danmar
commented
Oct 13, 2023
However that only checks the source code. It does not check the Makefile dependencies as far as I know. @Meiye-lj |
danmar
commented
Oct 13, 2023
I close this for now. Feel free to reopen if results has been fixed. |
Meiye-lj
commented
Oct 14, 2023
via email
Thank you! Our work is not yet public. When it becomes public. I'd be happy to share it. We re-detected cppcheck based on its most recent commits, and we found that since commit ID afe05d0, cppcheck no longer has missing dependencies. For earlier versions, it looks like our detection of missing dependencies is not false positives. I'd be happy to share our test reports if you're interested.
----- 原始邮件 -----
发件人:Daniel Marjamäki ***@***.***>
收件人:danmar/cppcheck ***@***.***>
抄送人:Meiye-lj ***@***.***>, Mention ***@***.***>
主题:Re: [danmar/cppcheck] New makefile to fix dependency errors (PR #5546)
日期:2023年10月13日 18点38分
We are also include-what-you-use to make sure
However that only checks the source code. It does not check the Makefile dependencies as far as I know.
@Meiye-lj
I think your tool which seems to check Makefile dependencies IS interesting. But it seems more development is needed. Please share with us when it works properly. If it works well I would be happy to spread the word.. it could be a valuable complement to Cppcheck..
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***> |
firewave
commented
Oct 20, 2023
Sorry for the late reply. You should not be using hard-coded dependencies in a Makefile at all. You should generate the dependencies based on the source. This functionality is integrated with the GNU compilers GCC and Clang via the An example on how to integrate it into your Makefile can be found here: https://stackoverflow.com/a/313787/532627. That would be a proper future-proof fix instead of just adjusting the hard-coded ones. Modern build systems like meson or CMake have this integrated so you don't have to take care of it by yourself anymore (not sure about Autoconf). So there is actually no external tooling necessary to do this.
Our Makefile is also not leveraging built-in make rules which leads to unnecessary code which is not actually necessary. |
Hi,
Based on our study, we found some dependency errors. We have tried to fix them.