-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix #13993: simplecpp: bump to 1.4.4 #7650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ludviggunne
wants to merge
5
commits into
danmar:main
Choose a base branch
from
ludviggunne:13993
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6945b1a
fix #13993: simplecpp: bump to 1.4.4
ludviggunne 1497f5f
add some functional macros to qt.cfg
ludviggunne 2222a30
add wxCHECK_VERSION(...) to wxwidgets.cfg
ludviggunne ce8ab21
update inline suppression test
ludviggunne 054f4ad
update workflows
ludviggunne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually that is the way to do it: