Re: Problem in Bug 93240

On 01.09.2015 06:45, Shreyansh Gandhi wrote:
> Hi,
>
> I have been working on a patch for EasyHack 93240
> <https://bugs.documentfoundation.org/show_bug.cgi?id=93240>  and I have
> run into a problem.
> All the changes I'll describe are in the sw/source/filter/html directory.
>
> *svxcss1.hxx , svxcss1.cxx , htmlcss1.cxx:-*
>
>   * http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/svxcss1.hxx#201 . According
>     to the bug description, boost::ptr_vector<..> should be changed to
>     std::vector<std::unique_ptr<CSS1Selector> > . aSelectors would be
>     renamed to m_Selectors. 

yes

>   * http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/svxcss1.cxx#659 
>     . In SvcCSS1Parser::SelectorParsed, since pSelector (which is of
>     type CSS1Selector*) would be pushed back into m_Selectors now and
>     there is no conversion from a normal pointer to a unique_ptr,
>     pSelector itself would have to be a unique_ptr. Hence, the

that's not entirely true: there is no *implicit* conversion to
unique_ptr, but you can explicitly construct the unique_ptr just fine.

because the boost::ptr_containers have the same ownership semantics for
elements, it should be safe to convert to unique_ptr in every place
where an element is inserted into a std::container of std::unique_ptr.

>     StyleParsed method's prototype would also be changed.
>   * http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/htmlcss1.cxx#696 .
>     Is another implementation of SvxCSS1Parser::StyleParsed.
>
>
> Since SvxCSS1Parser inherits CSS1Parser, and the SelectorParsed method
> is overloaded, changes are also required in *parcss1.hxx and parcss1.cxx
> *, where ParseRule() calls SelectorParsed():-

best to first finish the patch to convert the container and then do this
refactoring in a follow-up patch:

>   * http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/parcss1.cxx#739 .
>     pSelector (which should become a unique_ptr ) is obtained from
>     ParseSelector() (whose return type should also change) and then
>     passed to SelectorParsed() 
>   * http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/parcss1.cxx#833 .
>     In ParseSelector(), pRoot, pNew and pLast would have to be
>     unique_ptr s (initialized to nullptr). Lines 882, 894, 911 and 924
>     would now have calls to std::move. The problem arises in the if
>     block starting at line 936, where pRoot and pLast are checked for
>     equality (which should fail for unique_ptr s) Also, pRoot and pLast
>     could both be assigned to the object owned by pNew, which is not
>     possible.

it looks like it should be possible if pLast remains a CSS1Selector*,
then use unique_ptr::get() to assign it.

then also convert CSS1Selector::pNext to unique_ptr, so the first one is
owned by pRoot variable and the subsequent ones are owned by the
preceding one's pNext member.