<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>Hi,<br>
</p>
<div class="moz-cite-prefix">On 1/30/20 12:02 PM, Mario Charest via
cfe-users wrote:<br>
</div>
<blockquote type="cite" cite="mid:CA+Na9K5EqFFShXDUh2OpBY7-px8v1Pb=ysOvOYWK3EAV_RFkTQ@mail.gmail.com">
<div dir="ltr">Hello,
<div><br>
</div>
<div>First post, be gentle ;-)</div>
<div><br>
</div>
<div>I'm trying to find a clean solution to an error message
that clang-tidy is giving, tried with 10 and 11)</div>
<div><br>
</div>
<div>This is the code:</div>
<div><br>
</div>
<div>struct Foo<br>
{<br>
Foo(const std::string &value) : m_name(value) {} <br>
Foo(std::string &&value) :
m_name(std::move(value)) {}<br>
std::string m_name;<br>
};<br>
</div>
<div><br>
</div>
<div>The message is :</div>
<div><br>
warning: pass by value and use std::move
[modernize-pass-by-value]<br>
Foo(const std::string &value) : m_name(value) {}<br>
^~~~~~~~~~~~~~~~~~~<br>
std::string std::move( )</div>
<div><br>
</div>
<div>I understand the logic behind the warning.
Unfortunately the solution cannot be apply because of the move
constructor. Won't compile. One might argue the move
constructor could be remove. But I did not make that post to
get into that. What I would like to know if it would make
sense to make clang-tidy smarter about this and not generate
that message if a move constructor is present ? <br>
</div>
</div>
</blockquote>
<p>First I need to clear up a possible misconception: nowhere in the
code above did you declare a <i>move constructor</i> (or a copy
constructor, come to that). Instead, you declared two constructors
for <tt>struct Foo</tt>, each taking a single, non-defaulted
argument. The difference is that <tt>value</tt> is a <tt>std::string</tt>,
not a <tt>Foo</tt>. The copy and move constructors—and
assignments, and destructor—are implicitly defined for you by the
compiler, and this is the desired behavior in most circumstances.<br>
</p>
<p>The correct form of Foo according to current best practice for
modern C++ is:</p>
<pre>struct Foo
{
explicit Foo(std::string value) : m_name(std::move(value)) {}
////////////////////////////////////////////////////////////////////////
// This is not necessary, and causes a compile error due to ambiguity
// with the above:
// explicit Foo(std::string &&value) : m_name(std::move(value)) {}
////////////////////////////////////////////////////////////////////////
private:
std::string m_name;
};
</pre>
<p>This satisfies <tt>clang-tidy</tt> 9.0.1 with the following
invocation:</p>
<pre>/usr/local/opt/llvm/bin/clang-tidy --checks=* test-clang-tidy.cc -- -stdlib=libc++ -std=c++2a -Werror -Wall -Wextra -O3 -g
</pre>
<p>The single explicitly-declared constructor will quite happily
take a <tt>string</tt> by value, by <tt>const</tt> reference or
by r-value reference, rendering the second form in your original
example redundant. Trust the optimizer to avoid unnecessary copy
operations.<br>
</p>
<p>Please let me know if I can clarify anything further.</p>
<p>Best,</p>
<p>Chris.<br>
</p>
<blockquote type="cite" cite="mid:CA+Na9K5EqFFShXDUh2OpBY7-px8v1Pb=ysOvOYWK3EAV_RFkTQ@mail.gmail.com">
<div dir="ltr">
<div>Regards,</div>
<div><br>
</div>
<div>- Mario</div>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-pre" wrap="">_______________________________________________
cfe-users mailing list
<a class="moz-txt-link-abbreviated" href="mailto:cfe-users@lists.llvm.org">cfe-users@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dusers&d=DwIGaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=kj1IkzpbbOO6DafO4zbQ7w&m=z_bv0PrnNik_vXaj8COaotyKvyzuJ3JX5et9UI9udIE&s=AIR-R23WPe-HxbSgg_PesuBxJQxT6XAsgKkWRY_GI68&e=">https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dusers&d=DwIGaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=kj1IkzpbbOO6DafO4zbQ7w&m=z_bv0PrnNik_vXaj8COaotyKvyzuJ3JX5et9UI9udIE&s=AIR-R23WPe-HxbSgg_PesuBxJQxT6XAsgKkWRY_GI68&e=</a>
</pre>
</blockquote>
</body>
</html>