Make more signals non-const
ClosedPublic

Authored by kossebau on Nov 23 2017, 12:28 AM.

Details

Summary

Not sure exactly what is wrong with const signals, besides
QMetaObject::activate(...) wanting a non-const this argument.

Collected signals are from a recent clazy run, so changing these will at least
result in less noisy future clazy run ;)
And perhaps consistency in general signal non-constness.

The Clazy const-signal-or-slot check docs only claims
"For signals, it's just pointless to mark them as const."
without giving any reasoning.

So dumping patch as RFC, to be either moved to /dev/null or applied.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Nov 23 2017, 12:28 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptNov 23 2017, 12:28 AM
apol accepted this revision.Nov 23 2017, 1:02 AM
apol added a subscriber: apol.

LGTM, the only weird thing you had to touch was already doing weird things...

This revision is now accepted and ready to land.Nov 23 2017, 1:02 AM
kfunk accepted this revision.Nov 23 2017, 8:24 AM

Thanks for review, but... any comment on why signals should be non-const? :)

This revision was automatically updated to reflect the committed changes.

Even though, virtual signals ?!

brauch added a subscriber: brauch.Nov 23 2017, 10:45 AM

Well conceptually, a signal is only a signature. The const has no meaning conceptually (what would be the difference between a const and a non-const signal). So it makes sense to pick a normalized form -- the non-const one.

Practically, the signal gets implemented by the moc and ... does things (call slots), so it's not really a const operation because the slots are also potentially non-const ...

Even though, virtual signals ?!

Yes, that trick is needed because even in Qt5 no-one has added support to Qt's interfaces logic to allow requiring implementors to provide certain signals (like wanted when interfaces e.g. should provide a property with setter, getter, notifier).

Well conceptually, a signal is only a signature. The const has no meaning conceptually (what would be the difference between a const and a non-const signal). So it makes sense to pick a normalized form -- the non-const one.

Practically, the signal gets implemented by the moc and ... does things (call slots), so it's not really a const operation because the slots are also potentially non-const ...

Well, one could say the signal method itself does not call those slots. It is rather the one who connects the slots to the signal. Does not completely convince me :)

Another reasoning might be that signals are only needed when a state changes (why else should be a notification done). So the method emitting a signal should be non-const itself, and the signals being non-const enforces that (at least to the point where people are not wildly applying const_cast<>).