lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Conversion from class html::text


From: Vadim Zeitlin
Subject: Re: [lmi] Conversion from class html::text
Date: Sat, 3 Feb 2018 13:24:29 +0100

On Fri, 2 Feb 2018 16:18:51 +0000 Greg Chicares <address@hidden> wrote:

GC> Vadim--Consider the following inline comment on class text in 'html.hpp':
GC> 
GC> /// As it still needs to be converted to a string sooner or later to be 
really
GC> /// used, it does provide a conversion -- but it can be used only once.
GC> 
GC> What function provides that conversion? I'm guessing it's not this one:
GC>     std::string const& as_html() const&
GC> because I imagine that a const function can be used more than once;
GC> so is it this one:
GC>     std::string&& as_html() &&
GC> ?

 Yes, exactly. The "&&" makes this function "ref-qualified", just as "&" or
"const&" (different from "const"!) in the same location would, and means
that it can be called only on r-values, e.g. temporaries.

 Of course, this doesn't change the fact that the comment you refer to
above is wrong/out of date: having only this "once-only" conversion proved
to be too limiting, e.g. I couldn't use it in pdf_writer_wx::output_html(),
so I ended up by adding the "const&"-qualified overload above to allow
converting a non-temporary object to a string, so the comment should be
updated -- would you like me to do it or would you prefer to do it
yourself?

 Note that having r-value-ref-qualified overload is still useful because it
avoids copying (potentially not very small) strings in the most common use
case. And, in fact, I wonder if it might make sense to change
pdf_writer_wx::output_html() to take html::text by r-value reference as we
only ever pass temporary objects to it anyhow and then remove the "const&"
overload, restoring the comment validity. I.e. apply this (not really
tested yet) patch:

---------------------------------- >8 --------------------------------------
diff --git a/html.hpp b/html.hpp
index 3493f77d6..ac6e7e5ab 100644
--- a/html.hpp
+++ b/html.hpp
@@ -115,11 +115,6 @@ class text
         return *this;
     }
 
-    std::string const& as_html() const&
-    {
-        return m_html;
-    }
-
     std::string&& as_html() &&
     {
         return std::move(m_html);
diff --git a/pdf_writer_wx.cpp b/pdf_writer_wx.cpp
index 8a9228a80..66bca26e6 100644
--- a/pdf_writer_wx.cpp
+++ b/pdf_writer_wx.cpp
@@ -172,7 +172,7 @@
     (int x
     ,int y
     ,int width
-    ,html::text const& html
+    ,html::text&& html
     ,enum_output_mode output_mode
     )
 {
@@ -180,7 +180,7 @@
     // font which is changed by rendering the HTML contents.
     wxDCFontChanger preserve_font(pdf_dc_, wxFont());
 
-    auto const html_str = wxString::FromUTF8(html.as_html());
+    auto const html_str = wxString::FromUTF8(std::move(html).as_html());
     std::unique_ptr<wxHtmlContainerCell> const cell
         (static_cast<wxHtmlContainerCell*>(html_parser_.Parse(html_str))
         );
diff --git a/pdf_writer_wx.hpp b/pdf_writer_wx.hpp
index 3054fd700..7cb748334 100644
--- a/pdf_writer_wx.hpp
+++ b/pdf_writer_wx.hpp
@@ -59,7 +59,7 @@ class pdf_writer_wx
         (int x
         ,int y
         ,int width
-        ,html::text const& html
+        ,html::text&& html
         ,enum_output_mode output_mode = e_output_normal
         );
 
---------------------------------- >8 --------------------------------------

 What do you think, is it worth using a slightly unusual signature for
output_html() to restore the "convertible to string only once" property of
html::text and, incidentally, to avoid a bunch of string copies when
calling it?

VZ

P.S. I'm cc'ing this to you directly, but the list seems to work fine for
     me, i.e. I received the original message via it immediately when it
     was sent (but wasn't there to reply to it) and also received your
     reply in the PDF generation performance thread (both on and off list).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]