> On Nov. 10, 2014, 9:41 p.m., David Faure wrote: > > lib/konq/src/konq_historyentry.h, line 57 > > <https://git.reviewboard.kde.org/r/121080/diff/1/?file=327432#file327432line57> > > > > const ref, no? > > Andrius da Costa Ribas wrote: > before I try to fix the pending issues: what are we going to decide? > > - Should we create KDE_DUMMY_QHASH_FUNCTION macro again? (which header) > - Should it apply to MSVC-only or should it be ifdef-free? > > David Faure wrote: > Clearly this is not KDE specific. Any QList<custom type> requires this on > MSVC, right? Then I would strongly suggest a solution within Qt itself, *if* > a central solution such as a macro is indeed needed. But thinking about it, a > one-line dummy impl that returns 0 doesn't really seem worth a macro to me. > I.e. why not do like qitemselectionmodel.h does, everywhere where this is > needed? > > But I am still surprised that Qt only needs this in qitemselectionmodel.h > Take this for instance: > src/corelib/io/qstorageinfo.h: static QList<QStorageInfo> > mountedVolumes(); > Why doesn't this require a qHash(QStorageInfo)? > If I explicitly call toSet() on such a list, I do get a compile error (on > Linux) due to the missing qHash implementation. > http://www.davidfaure.fr/2015/storageview.diff > So MSVC doesn't *always* instanciate the toSet() method, but only in some > cases? > Or are we looking at an old MSVC issue which doesn't exist anymore with > more recent MSVC versions? i.e. did you try removing this block (in > konq_historyentry.h) altogether to check it's still needed? > > Andrius da Costa Ribas wrote: > There is some historic reference here: > http://marc.info/?l=kde-core-devel&m=113069150408826&w=2 > > MSVC 2013 still has the same beahviour.
I know it's because MSVC instanciates all methods. This is why I don't understand why it works for e.g. QStorageInfo. Sounds to me like further research is needed, I don't like fixes where we don't really understand what's going on. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121080/#review70214 ----------------------------------------------------------- On Nov. 8, 2014, 10:26 p.m., Andrius da Costa Ribas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121080/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2014, 10:26 p.m.) > > > Review request for KDE Base Apps, KDE Frameworks and kdewin. > > > Repository: kde-baseapps > > > Description > ------- > > Since we're not using kdemacros.h here anymore, KDE_DUMMY_QHASH_FUNCTION is > not available. Implement it instead. > > > Diffs > ----- > > lib/konq/src/konq_historyentry.h de34d6b > > Diff: https://git.reviewboard.kde.org/r/121080/diff/ > > > Testing > ------- > > It builds (MSVC2013 - 64bit) after this patch (along other patches I'm > sending to review today). Kdebase-apps is still not very functional, though > (missing icons and weird UI). > > > Thanks, > > Andrius da Costa Ribas > >
_______________________________________________ Kde-windows mailing list Kde-windows@kde.org https://mail.kde.org/mailman/listinfo/kde-windows
