[cfe-users] clang-tidy

[Mario Charest via cfe-users]

Le mer. 5 févr. 2020 09 h 48, Chris Green <greenc at fnal.gov> a écrit :

> Hi,
> On 1/30/20 12:02 PM, Mario Charest via cfe-users wrote:
>
> Hello,
>
> First post, be gentle ;-)
>
> I'm trying to find a clean solution to an error message that clang-tidy is
> giving, tried with 10 and 11)
>
> This is the code:
>
> struct Foo
> {
>     Foo(const std::string &value) : m_name(value) {}
>     Foo(std::string &&value) : m_name(std::move(value)) {}
>     std::string m_name;
> };
>
> The message is :
>
> warning: pass by value and use std::move [modernize-pass-by-value]
>     Foo(const std::string &value) : m_name(value) {}
>         ^~~~~~~~~~~~~~~~~~~
>         std::string                        std::move( )
>
> 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 ?
>
> First I need to clear up a possible misconception: nowhere in the code
> above did you declare a *move constructor* (or a copy constructor, come
> to that). Instead, you declared two constructors for struct Foo, each
> taking a single, non-defaulted argument. The difference is that value is
> a std::string, not a Foo. 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.
>
> The correct form of Foo according to current best practice for modern C++
> is:
>
> 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;
> };
>
> This satisfies clang-tidy 9.0.1 with the following invocation:
>
> /usr/local/opt/llvm/bin/clang-tidy --checks=* test-clang-tidy.cc  -- -stdlib=libc++ -std=c++2a -Werror -Wall -Wextra -O3 -g
>
> The single explicitly-declared constructor will quite happily take a
> string by value, by const reference or by r-value reference, rendering
> the second form in your original example redundant. Trust the optimizer to
> avoid unnecessary copy operations.
>
> Please let me know if I can clarify anything further.
>
> Best,
>
> Chris.
>
> Regards,
>
> Thanks Chris for the clarification on me terminology. That being said the
optimizer can only do its magic if the constructor is inlined. If it's in
definition and declaration are in different files, it won't work hence for
maximum performance both constructors are required.

>
> - Mario
>
> _______________________________________________
> cfe-users mailing listcfe-users at lists.llvm.orghttps://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=
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-users/attachments/20200205/fc7dfbec/attachment-0001.html>