Review Request 121080: Replace KDE_DUMMY_QHASH_FUNCTION.

[David Faure]

> 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?

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?


- 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20150418/179a937a/attachment.htm>