gcc -Woverloaded-virtual

[Aurélien Gâteau]

David Faure wrote:

> I fixed the slotInfoMessage warning a better way (moving the slot to the
> private class since it's unrelated to the one in the base class; I'll
> commit tomorrow), but I didn't have a fix for addSubjob/removeSubjob so
> I'm glad you raise this. Your post made me realize the argument-type
> difference... the fact that addSubjob takes a KIO::Job and removeSubjob
> takes a KJob is quite illogical. I would use KJob for both (since it's
> virtual anyway) and I would handle that correctly in the code (calling the
> KJob:: methods and not doing the KIO-specific stuff). I.e. allowing normal
> KJobs as children of KIO::Jobs instead of half-forbidding like now. Can
> you make that change?

I gave it a try, attached is the result, but I don't think it's the right
way to go.

First it changes addSubjob() signature (KCompositeJob::addSubjob() returns a
bool, while KIO::addSubjob() used to return nothing).

Second, more critical, the split of addSubjob() in two methods, one with the
inheritMetaData parameter and the other without it is dangerous, because
now in KIO::Job we have these two methods:

    virtual bool addSubjob(KJob*, bool inheritMetaData);       /* 1 */
    bool addSubjob(KJob* job) { return addSubJob(job, true); } /* 2 */

But (2) is now virtual, because it matches the signature of
KCompositeJob::addSubjob(). I am afraid this will lead to problems if one
is overloaded and not the other.

(Note that in this case there is no more overload warning, since we are
correctly reimplementing addSubjob).

A better fix IMO would be to get rid of (1), make (2) not inherit meta data
and add a method like this:

    void mergeMetaDataFromJob(KJob* job)

But it means all code which relies on KIO::Job::addSubjob() merging meta
data needs to be fixed.

Another solution is to make inheritMetaData a mandatory option, and get rid
of the overload warning with a private "using" line. But that means adding
the boolean to all code calling addSubjob().

In other words, I leave it up to you. You probably know KIO code much better
than me :-)

Aurélien
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kdelibs_fix_overloaded_warnings_jobclasses2.diff
Type: text/x-diff
Size: 2803 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20071125/58b3639a/attachment.diff>