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

[Greg Chicares]

On 2018-02-03 12:24, Vadim Zeitlin wrote:
> 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.

I'm still not fluent in this aspect of the modern language, so I need to
keep studying it. I've read these papers:
    A Brief Introduction to Rvalue References
  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2027.html
    Extending move semantics to *this
  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2439.htm
again, along with the relevant parts of the latest TC++PL and some of
Meyers's writings. It's easy to find introductory materials like those,
but I can't find much good non-introductory documentation.

I tend to see anything like:
  void foo(some_type some_argument); // Must be called only once!
                                        ^^^^^^^^^^^^^^^^^^^^^^^^ AARRGH
as a pitfall, and to wonder whether it should either be avoided altogether,
used only in restricted situations (like writing low-level "library" code),
or at least surrounded by a safety fence. I understand the motivations in
general (perfect forwarding; swap() without copying), and in this case I
can well imagine that a class that traffics in large strings may benefit
from eliminating copies. But is this actually unsafe?

If I understand this correctly, it's not unsafe because
    std::string&& as_html() && // Must not be called more than once!
_cannot_ be called more than once: the ref-qualifier '&&' prevents that.
For example, here:
    bool test_variable(std::string const& name) const
    {
        auto const z = expand_html(name).as_html();
it's not possible to call as_html() && more than once on the temporary:
if it weren't a temporary, the ref-qualifier would make that illegal; and
because it's a temporary, it doesn't have a name and we can't write a
subsequent expression that would use it. Is that reasoning valid?

>  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,

[...that "const&"-qualified overload is discussed later...]

> so the comment should be
> updated -- would you like me to do it or would you prefer to do it
> yourself?

Now I'm thinking that it's only the comment (and not the '&&'-qualified
function itself) that caused me discomfort. If my reasoning above is
correct, then no cautionary "don't do this" comment is needed:

 /// As it still needs to be converted to a string sooner or later to be really
-/// used, it does provide a conversion -- but it can be used only once.
+/// used, it does provide a conversion, as_html().

>  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:

In that patch, removing the "const&" overload requires only changing the
type of one argument to pdf_writer_wx::output_html():

-    ,html::text const& html
+    ,html::text&& html

and consequently adding std::move() here:

-    auto const html_str = wxString::FromUTF8(html.as_html());
+    auto const html_str = wxString::FromUTF8(std::move(html).as_html());

so it's a simplification, and I see nothing wrong with that, unless...

>  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?

I still balk at calling it "convertible only once"; I'd rather call
this the "usable only with temporaries" property (once again crucially
assuming that my reasoning about that above is correct).

I'd say the big advantage is eliminating the as_html() const& overload
for simplicity. Avoiding a string copy in this one isolated instance
might have no noticeable effect, but we're already avoiding them in
many other places where the cumulative savings could be noticeable.

And now we come to the "unusual signature" question, which I can't
really answer because I don't know what's idiomatic--but does anyone?
In 1986, the signature now in HEAD looked weird:

int pdf_writer_wx::output_html
    (int x
    ,int y
    ,int width
    ,html::text const& html
    ,enum_output_mode output_mode
    )

Does the change suggested:

-    ,html::text const& html
+    ,html::text&& html

look really odd today? Why? Would it be less odd if this were the
first parameter in that list of five? Or if the other arguments
were combined and passed as a struct, e.g.? How about this
signature for std::forward_list [23.3.4.5]:
  iterator insert_after(const_iterator position, T&& x);

> 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).

OTOH, I don't imagine you got either of the "This is a test"
messages that I sent to this list from two different accounts.
For the time being, if you don't mind, I'll CC you, and ask you
to continue to CC me (I received only your CC of the message
replied to here--but no copy with any gnu.org server in the
headers). Meanwhile, I'll monitor the html archives and see what
comes through at another address that I've subscribed to this list.