On Apr 29, 2011 6:26 PM, "David Faure" <fa...@kde.org> wrote: > > On Friday 29 April 2011, Jeremy Whiting wrote: > > Since David added QT_USE_FAST_OPERATOR_PLUS to our build rules for kdelibs, > > it wont build here. > > Oops? > > > I've got a commit on the whiting/fixQByteArrays that > > fixes this, but I'd like someone to review it before I merge to master. > > Anyone got a moment to test and review it? > > Reviewboard is a better tool for this, but okay, I'm not in a position to > complain, as the guy with the hat of shame for breaking the build :) >
It could be because arch recently switched to gcc 4.6. Could also be because I'm trying to use qt 4.8 from git. I'll take a try at your suggestions when I get up in the morning. Jeremy > - QString testpath = "file://" + path + "/kdelibs/khtml/test/"; > + QString testpath = QString("file://" + path + "/kdelibs/khtml/test/"); > Can be done simpler as > QString testpath("file://" + path + "/kdelibs/khtml/test/"); > > Surprisingly (or rather unsurprisingly, since I did test before committing), > it compiles without this change. gcc-4.4.5 here, kubuntu; which compiler do > you use? > > This is a case of an explicit conversion to QString so in theory it should > just work (unlike all the template cases I fixed in my commit), but maybe it > breaks on some compilers because the first arg is a char litteral. > > I guess a more logical change, then, could be > QString testpath = QString("file://") + path + "..."; > > The other changes are related to QByteArray, which leaves me completely > stumped. Fast operator plus is for QString only. How can m_tokens[id+uuid] > break, when id and uuid are bytearrays? Oh, m_tokens takes a QString as key, > blah. > Please test and apply the real fix: > - QMap<QString, QByteArray> m_tokens; > + QMap<QByteArray, QByteArray> m_tokens; > AFAICS the key is used as a bytearray everywhere in > kdelibs/plasnma/private/serviceprovider.cpp > > For the last change: > - removeDir(QLatin1String("packageRoot/" + dir.toLatin1() + > "/contents/code")); > + removeDir(QLatin1String(QByteArray("packageRoot/" + dir.toLatin1() + > "/contents/code"))); > > I think it would be simpler to write > removeDir(QLatin1String("packageRoot/") + dir + "/contents/code")); > (or QString::fromLatin1 instead of QLatin1String, if necessary) > (and maybe with a QLatin1String around the last arg, if this code uses "no > cast from ascii". The compiler will tell you ;) ). > > -- > David Faure, fa...@kde.org, http://www.davidfaure.fr > Sponsored by Nokia to work on KDE, incl. Konqueror ( http://www.konqueror.org).