woboq / verdigris

Qt without moc: set of macros to use Qt without needing moc
https://woboq.com/blog/verdigris-qt-without-moc.html
GNU Lesser General Public License v3.0
643 stars 60 forks source link

Coverity Scan parsing errors #39

Open DrMcCoy opened 6 years ago

DrMcCoy commented 6 years ago

Coverity Scan seems to choke on a few of Verdigris constructions, entirely skipping certain compilation units with a "Unrecoverable parse warning (PARSE_ERROR)" note.

The repository in question is https://github.com/xoreos/phaethon , the Coverity Scan page is https://scan.coverity.com/projects/xoreos-phaethon (the issues themselves aren't viewable without registering and poking me for access to that project, though). We're using Verdigris from git commit 11ee05bff379f88240c4e6ea07b881c18940d4b7.

Below is a copy of the issues.

I'll gladly try to provide more information if you need it, but Coverity Scan itself doesn't give me more, I think.

Neither gcc nor clang, neither locally nor on Travis CI, seem to have a problem with the code, only Coverity Scan throws there. The scan build itself was build on Travis CI with gcc and uploaded to Coverity Scan by the Travis VM.

ogoffart commented 6 years ago

Looks like some problem in coverity. I wonder if the best way would not to change the definition of the macro with coverity so it does not expand to create a QMetaObject

i.e:

#ifdef COVERITY  //is there a macro like this?
#define W_OBJECT_IMPL(...) /* nothing */
#define W_SIGNAL(...) ;
//...
DrMcCoy commented 6 years ago

AFAIK, Coverity defines the __COVERITY__ macro during its compilation step.

But I'm not sure that this would work this way as you suggest here. Coverity is a static analyzer, and I would assume that it still tries to evaluate both branches of a such a macro.

And besides, this is really not what I want. I want Coverity to check the code as-is.

If you say that this is all legal standard C++ you're using and not some non-standard extensions, the fault is clearly with Coverity there. I guess I should contact them about it.

I just hoped that you might say "yeah, this is broken, let me fix it", or something. :P

jcelerier commented 6 years ago

If you say that this is all legal standard C++ you're using and not some non-standard extensions, the fault is clearly with Coverity there. I guess I should contact them about it.

IIRC coverity uses a relatively old version of clang for their C++ parsing.

ogoffart commented 6 years ago

This is all legal standard C++. But this uses fairly advanced constexpr/template meta-programming, which tends to expose bugs in compilers.

In particular, I think these issues are bugs.

incomplete_type_not_allowed: incomplete type is not allowed [...] during instantiation of class "w_internal::w_number [with N=]"

There is no incomplete types there. N has a default value of 255. Maybe it got confused because the inheritence tree is quite big.

"auto_variable_in_own_initializer: a variable declared with an auto type specifier cannot appear in its own initializer"

I am not aware of any variable using themself in their initializer.

no_user_defined_conversion: no suitable user-defined conversion from "w_internal::w_number<255>" to "w_internal::w_number<0>" exists

w_internal::w_number<255> inherits from w_internal::w_number<254> which inherit from w_internal::w_number<253> which inherit from ... which inherit from w_internal::w_number<0>, so the conversion should be possible. Maybe related to the first error.

no_matching_function: no instance of "w_SignalState" matches the argument list [...] argument types are: (w_internal::w_number<255>, [...]

Seems related to the previous error.

I would assume that it still tries to evaluate both branches of a such a macro.

I would very much doubt that coverity tries to evaluate both branch of preprocessor #if. And especially not the __COVERITY__ one.

And besides, this is really not what I want. I want Coverity to check the code as-is.

All what these macro does is to generate the QMetaObject at compile time. It has no effect on the rest of code, and should therefore let coverity to continue to report the same errors. (maybe we ned to be carefull to keep the signal implementation and the metaObject() implementation, so coverity can see that. But it most certainly does not need to see the QMetaObject itself)