Skip to content

Conversation

@firewave
Copy link
Collaborator

@firewave firewave commented Oct 9, 2023

No description provided.

@firewave firewave changed the title qt.cfg: implemented Q_OBJECT to get rid of symbolDatabaseWarning in selfcheck fixed #12056 - qt.cfg: implemented Q_OBJECT to get rid of symbolDatabaseWarning in selfcheck Oct 9, 2023
@chrchr-github
Copy link
Collaborator

I still wonder why we scan moc_*.cpp files in the first place.

@firewave
Copy link
Collaborator Author

firewave commented Oct 9, 2023

I still wonder why we scan moc_*.cpp files in the first place.

True, we cannot fix any warnings in there. So it's rather dogfooding/bootstrapping for those files.

But it might expose actual issues in the generated code as well as highlight analysis issues (the suppressed simplifyUsing is definitely such). It might also be possible that user code could be injected into those functions. I am not familiar with Qt at all.

@firewave firewave changed the title fixed #12056 - qt.cfg: implemented Q_OBJECT to get rid of symbolDatabaseWarning in selfcheck fixed #12056 - qt.cfg: implemented Q_OBJECT to get rid of symbolDatabaseWarning in selfcheck Oct 9, 2023
@firewave
Copy link
Collaborator Author

cmake.output.notest/gui/cppcheck-gui_autogen/EWIEGA46WW/moc_aboutdialog.cpp:76:0: style: The function 'metaObject' is never used. [unusedFunction]
const QMetaObject *AboutDialog::metaObject() const
^

It is strange that we are only getting a single warning. That should be reported for each of the classes based on Q_OBJECT.

@firewave
Copy link
Collaborator Author

It is strange that we are only getting a single warning. That should be reported for each of the classes based on Q_OBJECT.

It appears these are not reported because of the simplifyUsing issues in the other moc_*.cpp files.

@firewave firewave marked this pull request as ready for review October 10, 2023 20:24
@firewave
Copy link
Collaborator Author

It is strange that we are only getting a single warning. That should be reported for each of the classes based on Q_OBJECT.

It appears these are not reported because of the simplifyUsing issues in the other moc_*.cpp files.

Still we should be also getting unusedFunction warnings for the three other functions in Q_OBJECT. Will investigate.

@firewave
Copy link
Collaborator Author

Still we should be also getting unusedFunction warnings for the three other functions in Q_OBJECT. Will investigate.

Possibly the same simplifyUsing issue with the code not being processed.

@firewave
Copy link
Collaborator Author

Still we should be also getting unusedFunction warnings for the three other functions in Q_OBJECT. Will investigate.

Possibly the same simplifyUsing issue with the code not being processed.

Nope. One is actually used and the other ones are not reported since they call the base class implementation with the same name.

@firewave
Copy link
Collaborator Author

Anything still to do here?

@chrchr-github
Copy link
Collaborator

Given that Q_OBJECT is just a tag for the moc compiler, I see no upside for the user in defining it.
If we really want to scan moc* files, can't we define it locally for the analysis?

@firewave
Copy link
Collaborator Author

*We' should scan them as integration test (they actually exposed several bugs).

Also it could happen that the generated code contains bugs we might find. As mentioned before I don't know if user-code can be injected into the generated one. Also it avoids additional issues. And I doubt most people will exclude the moc_*.cpp files when passing a compilation database.

@firewave
Copy link
Collaborator Author

It is actually essential for the unusedFunction or we will get lots of false positives because the calls of functions from the ui_*.h are not being seen.

That is also an annoying issue within CLion which shows me lots of unused GUI functions because the generated files are not being considered.

@danmar
Copy link
Owner

danmar commented Nov 19, 2023

I am not sure if we want to model the moc.

I have the feeling the CheckUnusedFunction could relax its checking in classes that has a Q_OBJECT.

It seems to me it would be good if we could check qt code without having the generated moc_*.cpp and ui_*.h files... without getting false positives.

@firewave
Copy link
Collaborator Author

In terms what this tries to achieve is similar to the markup handling in the libraries. The main difference that this is just a define and not a separate feature implemented just for that case.

Currently this would require interaction from each user to work and we usually go for a small/no configuration and this would be in line with that.

To have to working better out-of-the-box would require a two-stage macro handling as suggested here: https://trac.cppcheck.net/ticket/8956#comment:10.

@firewave
Copy link
Collaborator Author

There is also the slots handling. Which clang-tidy does not handle that which leads to lots of false positives and also required us to disable a couple of checks. (But now that you can inherit the parent config we could enable those again...)

@firewave firewave merged commit d1b42d0 into danmar:main Nov 26, 2023
@firewave firewave deleted the qt-object branch November 26, 2023 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants