fix #13993: simplecpp: bump to 1.4.4#7650
fix #13993: simplecpp: bump to 1.4.4#7650ludviggunne wants to merge 5 commits intocppcheck-opensource:mainfrom
Conversation
| @@ -1,11 +1,77 @@ | |||
| file(GLOB hdrs "*.h") | |||
There was a problem hiding this comment.
Only the sources need to be updated.
There was a problem hiding this comment.
It might make sense to add a small script to the tools folder which pulls the latest simplecpp source and copies it over into the folder.
There was a problem hiding this comment.
Is a bash script fine for that or should it be cross platform?
|
There are some failures now due to undefined QT macros, I'm not quite sure how to deal with these. Add them to |
Add the library if it it is missing in the context (either via Since those are version check macros it would be great if we implement those so the configuration detection could come up with something that would check both cases but that is out of scope. @chrchr-github |
|
I've updated some cfg-files. |
|
In XML you need to encode |
390e372 to
aa3ea43
Compare
| <define name="QT_FORWARD_DECLARE_CLASS(name)" value="class name;"/> | ||
| <define name="QT_FORWARD_DECLARE_STRUCT(name)" value="struct name;"/> | ||
| <define name="QT_VERSION_CHECK(major, minor, patch)" value="((((major) & 0xff) << 16) | (((minor) & 0xff) << 8) | ((patch) & 0xff))"/> | ||
| <define name="QT_CONFIG(feature)" value="(1/QT_FEATURE_##feature == 1)"/> |
There was a problem hiding this comment.
I am not sure about this.. if the QT_FEATURE is not defined then there will be a unclear preprocessorErrorDirective warning it seems? I fear that it will force our users to configure a whole bunch of QT_FEATURE macros.. how about (QT_FEATURE_##feature == 1) instead?
There was a problem hiding this comment.
It feels like this is out of scope for this PR but;
I think it can be unfortunate to copy paste macros as is from library headers if it will create stronger dependency on library internals.. that will more or less remove the benefit of the library and the headers could have been included directly instead to get the real macros instead..
There was a problem hiding this comment.
The idea with having a division by zero is that it will generate a compile error if the macro isn't defined. But I suppose that's not something cppcheck needs to do. So just checking ==1 might be better.
I'm don't know the qt build system very well, but if the feature macros will be added in a compilation database, that should be fine. But then I guess I shouldn't have had to add those manually in the CI... I can try to reproduce and create a ticket.
There was a problem hiding this comment.
I think it can be unfortunate to copy paste macros as is from library headers if it will create stronger dependency on library internals.. that will more or less remove the benefit of the library and the headers could have been included directly instead to get the real macros instead..
copy and pasting from licensed headers has other implications. but let's not discuss that right now.
There is a way to getting the actual macro values pointed out here: https://trac.cppcheck.net/ticket/8956#comment:10. But that seems to be the approach for the internal defines. I did not find the ticket about source defined ones.
There was a problem hiding this comment.
Actually that is the way to do it:
$ gcc -E -dM $(pkg-config --cflags-only-I Qt6Core) a.cpp | grep QT_CONFIG
#define QT_CONFIG(feature) (1/QT_FEATURE_ ##feature == 1)
| if: false # TODO: fails with preprocessorErrorDirective - see #10667 | ||
| run: | | ||
| ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib --library=qt -D__CPPCHECK__ -D__GNUC__ -DQT_VERSION=0x060000 -DQ_MOC_OUTPUT_REVISION=69 -DQT_CHARTS_LIB -DQT_MOC_HAS_STRINGDATA --enable=unusedFunction --exception-handling -rp=. --project=cmake.output/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr | ||
| ./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib --library=qt -D__CPPCHECK__ -D__GNUC__ -DQT_VERSION=0x060000 -DQ_MOC_OUTPUT_REVISION=69 -DQT_CHARTS_LIB -DQT_MOC_HAS_STRINGDATA --enable=unusedFunction --exception-handling -rp=. --project=cmake.output/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr -DQT_FEATURE_shortcut -DQT_FEATURE_tooltip |
There was a problem hiding this comment.
hmm.. it's probably out of scope for this PR but I think the messy qt macros are unfortunate. For instance Q_MOC_OUTPUT_REVISION is not used directly in our code and shouldn't have to be defined manually. The QT_FEATURE_ macros should not have to be defined manually..
There was a problem hiding this comment.
That define is used by the generated code. And we need to define them manually because we do not use a project as input. But we already discussed that in another ticket.
be25de4 to
5a60f72
Compare
|
Should we just leave the library configurations untouched for this PR, and supply the macros on the command line in the CI for now? I feel like any other approach (such as the one where we parse the gcc output) is maybe a bit out of scope right now. Then again, now that simplecpp produces errors for undefined funcitonal macros, it might affect users who would also need to define these manually. @danmar @firewave |
|
They need to be added to the libraries because if we encounter this then users will also encounter it - and they will probably encounter even more than we do. It is also a bit unfortunate that we now have two different behaviors for unknown macros albeit in different contexts. Maybe those missing preprocessor macros should be reported via a separate error ID im simplecpp and be treated as |
|
There is now a different release that omits 4bbd1bf8e320471f2a46908c947251ed8aa18b0e. |
|
A later version has already been merged |
No description provided.