[vlc-devel] [PATCH] qt:soutchain: fix compilation with Clang 9

Alexandre Janniaux ajanni at videolabs.io
Mon Nov 4 16:02:28 CET 2019


Hi,

For my part, if I understood the issue correctly, I would write
something like:


qt: soutchain: make soutchain copyable

Qt requires container elements to be copyable, but class with const
members can't be copy-assignable. Remove the const values so that we
can copy SoutChain.

This patch fix the compilation with Clang 9.


I might have misunderstood the exact cause, but I hope it helps,

Regards,
--
Alexandre Janniaux
Videolabs

On Mon, Nov 04, 2019 at 01:48:04PM +0100, Steve Lhomme wrote:
> Will this do as commit log message ?
>
> Qt requires container elements to be copyable:
> https://bugreports.qt.io/browse/QTBUG-54685
> but we can't do that with const members.
>
>
> On 2019-11-04 13:41, Hugo Beauzée-Luyssen wrote:
> > Hi,
> >
> > On Mon, Nov 4, 2019, at 1:17 PM, Jérôme Froissart wrote:
> > > On Mon, 4 Nov 2019, 12:54 Steve Lhomme, <robux4 at ycbcr.xyz> wrote:
> > > > On 2019-11-04 12:42, Alexandre Janniaux wrote:
> > > >   > Hi,
> > > >   >
> > > >   > Maybe the commit message can be more explicit about what is the real
> > > >   > issue and how it is fixed. Otherwise, LGTM.
> > > >
> > > >   Probably but I don't understand the issue. I only understand the symptom.
> > >
> > > I hope my reply will be appropriate (maybe I won't explain anything you
> > > did not know, but since I wrote this class, this mail ringed a bell for
> > > me).
> > >
> > > It looks like QList::append( obj ) tries somewhere to assign a
> > > SoutModule (i.e. module1 = module2), but this assignment operator has
> > > not been defined manually (and is not generated automatically by the
> > > compiler since assigning this way would need to re-define member
> > > objects...that have been declared const, which is not possible by
> > > design).
> > >
> > > Usually, the solution would be to either:
> > > * find another method from QList that does not require the object to be
> > > assignable.
> >
> > I doubt this will ever be possible. Qt requires container elements to be copyable: https://bugreports.qt.io/browse/QTBUG-54685
> >
> > > * turn these const members as non-const
> >
> > I think this is the correct fix indeed. Actually it doesn't matter much if the member variables are const or not since they are private. The accessors are enough to mandate the object immutability IMO.
> >
> > > * define ourself a assignment operator (but it does not seem possible
> > > since there are const members anyway).
> > >
> >
> > I'm afraid you are correct, copy assignment won't work, and neither would move assignment.
> >
> > > I have not read the QList documentation right now, so unless we really
> > > do *not* want SoutModule to be assignable (and I think we have no
> > > reason not to want it), removing the 'const' like Steve did seems
> > > sensible.
> > >
> >
> > TL;DR: This patch looks good to me
> >
> > > Jérôme Froissart
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> >
> > --
> >    Hugo Beauzée-Luyssen
> >    hugo at beauzee.fr
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> >
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list