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

[Steve Lhomme]

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
>