<div dir="ltr">Hi Michael,<div><br></div><div>I just came across this: <a href="http://stackoverflow.com/questions/24221955/creating-a-unique-ptr-from-a-pointer">http://stackoverflow.com/questions/24221955/creating-a-unique-ptr-from-a-pointer</a> . Will it be safe to use this method to push_back into m_Selectors?</div><div><br></div><div>-Shreyansh</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 1, 2015 at 6:16 PM Michael Stahl <<a href="mailto:mstahl@redhat.com">mstahl@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 01.09.2015 06:45, Shreyansh Gandhi wrote:<br>
> Hi,<br>
><br>
> I have been working on a patch for EasyHack 93240<br>
> <<a href="https://bugs.documentfoundation.org/show_bug.cgi?id=93240" rel="noreferrer" target="_blank">https://bugs.documentfoundation.org/show_bug.cgi?id=93240</a>> and I have<br>
> run into a problem.<br>
> All the changes I'll describe are in the sw/source/filter/html directory.<br>
><br>
> *svxcss1.hxx , svxcss1.cxx , htmlcss1.cxx:-*<br>
><br>
> * <a href="http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/svxcss1.hxx#201" rel="noreferrer" target="_blank">http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/svxcss1.hxx#201</a> . According<br>
> to the bug description, boost::ptr_vector<..> should be changed to<br>
> std::vector<std::unique_ptr<CSS1Selector> > . aSelectors would be<br>
> renamed to m_Selectors.<br>
<br>
yes<br>
<br>
> * <a href="http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/svxcss1.cxx#659" rel="noreferrer" target="_blank">http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/svxcss1.cxx#659</a><br>
> . In SvcCSS1Parser::SelectorParsed, since pSelector (which is of<br>
> type CSS1Selector*) would be pushed back into m_Selectors now and<br>
> there is no conversion from a normal pointer to a unique_ptr,<br>
> pSelector itself would have to be a unique_ptr. Hence, the<br>
<br>
that's not entirely true: there is no *implicit* conversion to<br>
unique_ptr, but you can explicitly construct the unique_ptr just fine.<br>
<br>
because the boost::ptr_containers have the same ownership semantics for<br>
elements, it should be safe to convert to unique_ptr in every place<br>
where an element is inserted into a std::container of std::unique_ptr.<br>
<br>
> StyleParsed method's prototype would also be changed.<br>
> * <a href="http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/htmlcss1.cxx#696" rel="noreferrer" target="_blank">http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/htmlcss1.cxx#696</a> .<br>
> Is another implementation of SvxCSS1Parser::StyleParsed.<br>
><br>
><br>
> Since SvxCSS1Parser inherits CSS1Parser, and the SelectorParsed method<br>
> is overloaded, changes are also required in *parcss1.hxx and parcss1.cxx<br>
> *, where ParseRule() calls SelectorParsed():-<br>
<br>
best to first finish the patch to convert the container and then do this<br>
refactoring in a follow-up patch:<br>
<br>
> * <a href="http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/parcss1.cxx#739" rel="noreferrer" target="_blank">http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/parcss1.cxx#739</a> .<br>
> pSelector (which should become a unique_ptr ) is obtained from<br>
> ParseSelector() (whose return type should also change) and then<br>
> passed to SelectorParsed()<br>
> * <a href="http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/parcss1.cxx#833" rel="noreferrer" target="_blank">http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/parcss1.cxx#833</a> .<br>
> In ParseSelector(), pRoot, pNew and pLast would have to be<br>
> unique_ptr s (initialized to nullptr). Lines 882, 894, 911 and 924<br>
> would now have calls to std::move. The problem arises in the if<br>
> block starting at line 936, where pRoot and pLast are checked for<br>
> equality (which should fail for unique_ptr s) Also, pRoot and pLast<br>
> could both be assigned to the object owned by pNew, which is not<br>
> possible.<br>
<br>
it looks like it should be possible if pLast remains a CSS1Selector*,<br>
then use unique_ptr::get() to assign it.<br>
<br>
then also convert CSS1Selector::pNext to unique_ptr, so the first one is<br>
owned by pRoot variable and the subsequent ones are owned by the<br>
preceding one's pNext member.<br>
<br>
<br>
_______________________________________________<br>
LibreOffice mailing list<br>
<a href="mailto:LibreOffice@lists.freedesktop.org" target="_blank">LibreOffice@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/libreoffice" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/libreoffice</a><br>
</blockquote></div><div dir="ltr">-- <br></div><div dir="ltr">Regards,<div>Shreyansh Gandhi</div></div>