On Tuesday 22 November 2011, Stephen Kelly wrote: > 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).
I don't know that one. Are there docs somewhere ? I have also seen a I think Q_GADGET macro mentioned somewhere, but also didn't find docs. If they exist and require special handling, adding this special handling is fine IMO. > 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. Yes, it should work. I'm adding a test. > 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" This should work right now. I thought there is a test for this. Checking. > find #include "(.*).moc" -> generate moc for "\\1.cpp" > Was there a reason for preventing this? We can do that, but how can that work ? I mean, this will generate code for a class which is declated in a different source file, so it is unknown in the including file, no ? > >> 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. I think the error (it errors out, it's not a warning) is a good thing, otherwise it will break during compiling with a less descriptive error message. Alex -- 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