On 11/22/2011 06:20 PM, Alexander Neundorf wrote:
Using Qt5 or Qt4 ?

Qt4.

There seems to be several problems:

* KDE does include "foo.moc" if it wants the header file to be moc'd.
This is 'incorrect' compared to what qmake expects, which would be a
"moc_foo.cpp" include.
Yes.

* Some places in KDE use include "moc_foo.cpp" 'correctly', but there
are only about 10 occurrences. The current automoc tool seems to handle
that, as does cmake 2.8.6 apparently.
I thought about this. There are 4 constellations, and only one constellation
where things can break:

* foo.cpp includes neither foo.moc nor moc_foo.cpp. No problem.

Sort of. There are cases where the moc file must be included in the cpp file (see below).

But this removal of includes is what we should aim for in KDE. I think if the options are:

1) Introduce odd hacky KDE specific workarounds to CMake
2) Remove the problematic includes from KDE files in the frameworks branch

I will side with option 2). CMAKE_AUTOMOC is not used in KDE4. The feature is only really relevant to non KDE Qt4 users. Let's just 'fix the bug' in the frameworks iteration of kdelibs.

* if foo.cpp includes both foo.moc and moc_foo.cpp, then both foo.h and
foo.cpp will be moc'ed, and it doesn't matter which one is for which. No
problem.
It seems there is a test missing for this case.

Yes.

* if foo.cpp includes only foo.moc, then foo.cpp will be moc'ed. This may be
unnecessary, but shouldn't create problems. If foo.h also needs to be moc'ed,
it will be, and the resulting moc_foo.cpp will be included in the
target_automoc.cpp file. So it should also work.
This is Tests/QtAutomoc/codeeditor.cpp

Sort of. There are cases where the moc file must be included in the cpp file, because it needs definitions from the .h file. (eg, use of the Q_PRIVATE_SLOT macro).

If a KDE developer included foo.moc expecting that to be the result of moc'ing the .h file, the build will fail.

In this case, foo.cpp needs to be fixed to include "moc_foo.cpp" instead. I don't think CMake should introduce a hack for this.


* problematic case: if foo.cpp includes only moc_foo.cpp, then cmake will
check whether foo.cpp contains "Q_OBJECT". If it does, it will:
** fail with Qt5, stating that foo.cpp contains Q_OBJECT but does not include
foo.moc, so it can't work

I think this is fine.

** warn with Qt4 that it will run moc on foo.cpp instead of foo.h to generate
moc_foo.cpp
This is Tests/QtAutomoc/blub.cpp


* KDE does include "foo_test.moc" in foo_test.cpp to moc the test object
in foo_test.cpp. This is 'correct' compared to what qmake does.
Yes.

* KDE does eg, include "kjob_p.moc" in kjob.cpp to cause kjob_p.h to be
moc'd. The RestoreAutmocKDECompatibility branch introduces a commit
which makes this not work because the source file has a different basename.
Yes.
I did not expect somebody would be doing this.
I don't want to support this for Qt5.
I can add a fix with a warning for Qt4.

This is not uncommon in both KDE and in Qt itself, or any other project where it makes sense to put a QObject-inherited class in the _p.h as an internally used class. See for example, qplaintextedit.cpp:

#include "moc_qplaintextedit.cpp"
#include "moc_qplaintextedit_p.cpp"

This should be supported by the automoc feature.

I have also seen places (I think in QtDeclarative) where a foo.cpp file included the file moc_bar.cpp, though that is very uncommon. The logic that I would use would be something like:

find #include "moc_(.*).cpp" -> generate moc for "\\1.h"
find #include "(.*).moc" -> generate moc for "\\1.cpp"

Was there a reason for preventing this?

I think we need to have a list of exactly what cases should be
supported,
This is the current documentation of automoc in cmake. I was hoping it would
be relatively clear:

"If an #include statement like #include \"moc_foo.cpp\" is found,"
"the Q_OBJECT class declaration is expected in the header, and moc is"
"run on the header file."
"If an #include statement like #include \"foo.moc\" is found,"
"then a Q_OBJECT is expected in the current source file and moc"
"is run on the file itself."
"Additionally, all header files are parsed for Q_OBJECT macros,"
"and if found, moc is also executed on those files. The resulting"
"moc files, which are not included as shown above in any of the source"
"files are included in a generated<targetname>_automoc.cpp file, "
"which is compiled as part of the target."

This should be compatible to qmake, and this is what I'd like to support for
Qt5.

Nearly. As you can see, there are quite some differences compared to the qmake behavior.


and fix KDE to do the right thing. I think in many cases,
fixing KDE will just be removing the explicit include "foo.moc" which is
intended to run moc on foo.h, and let the automatic moc'ing do the work.

If any other 'KDE compatilbility' features are in cmQtAutomoc which
differ from qmake behavior, I really think it should be behind an option
which is off by default, which KDE turns on, and then actively ports
away from. Currently the 'KDE compatilbility' stuff is always on, which
is not an incentive to do things right or consistently.
Since it has been released with cmake 2.8.6, it can only be turned off by
default using a cmake policy. I'd like to avoid that, and instead support it
and issue a warning (for Qt4).

Currently the warning is also issued for Qt5, and the containsQ_OBJECT is called for Qt5 too. I would change that to also not search for Q_OBJECT or the warning for Qt5.

Right. I wouldn't have any issue with not making the KDE4 compatibility stuff active when using Qt5.

That means that we (KDE people) have to fix these kinds of things anyway in the KDE frameworks branch though.

I think there's more to do on this feature before master is backwards compatible with 2.8.6, even without KDE compat stuff.

Thanks,

Steve.

--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Reply via email to