Re: [cfe-users] clang-tidy
Hey Mario,
I logged a bug for this a long time ago:
https://bugs.llvm.org/show_bug.cgi?id=25319
>From what I can see, this still ain't solved.
It makes sense to register you onto that one.
On Thu, Jan 30, 2020, 19:02 Mario Charest via cfe-users <
cfe-users@lists.llvm.org> 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::stringstd::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 ?
>
> Regards,
>
> - Mario
> ___
> cfe-users mailing list
> cfe-users@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
>
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] clang-tidy
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,
- Mario
___
cfe-users mailing list
cfe-users@lists.llvm.org
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=
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] Silencing errors in TSAN
Hey Kyle,
This is a bug in your code, if you have a context switch after the creation
of the thread on an overloaded system, the close of the pipe can be closed
while writing or even before it.
Timing ain't a good way to synchronize threads. TSan is correct in flagging
this occurrence.
On Fri, Jan 24, 2020, 16:15 Kyle Edwards via cfe-users <
cfe-users@lists.llvm.org> wrote:
> Hello all,
>
> I am attempting to build OpenMPI with TSAN enabled (to TSAN-itize a
> project that uses OpenMPI), and am finding that OpenMPI throws lots of
> errors in TSAN (not surprising.) I am attempting to build OpenMPI with
> the following blacklist:
>
> src:*
>
> However, I am still getting errors when I run mpiexec. I've reduced it
> down to the following minimal example:
>
> --
>
> #include
>
> #include
>
> static int my_pipe[2];
>
> int main()
> {
> pipe(my_pipe);
> std::thread([]() {
> std::this_thread::sleep_for(std::chrono::seconds(1));
> close(my_pipe[1]);
> }).detach();
> write(my_pipe[1], "Hello", 5);
> std::this_thread::sleep_for(std::chrono::seconds(2));
> return 0;
> }
>
> --
>
> Even when I compile this with -fsanitize-blacklist=blacklist.txt, I
> still get a warning about a race condition. Is there some way I can
> silence this, preferably without patching OpenMPI? Is this a bug in
> TSAN?
>
> Kyle
> ___
> cfe-users mailing list
> cfe-users@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
>
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] Silencing errors in TSAN
On Wed, 2020-02-05 at 19:30 +0100, JVApen wrote: > Hey Kyle, > > This is a bug in your code, if you have a context switch after the > creation of the thread on an overloaded system, the close of the pipe > can be closed while writing or even before it. > > Timing ain't a good way to synchronize threads. TSan is correct in > flagging this occurrence. JVApen, Thanks for the response. I don't doubt that TSAN is correct. The problem is that this isn't my code. It's from OpenMPI, and fixing OpenMPI is beyond the scope of my work. I am attempting to build OpenMPI with TSAN enabled, so that applications that use it don't get false positives due to libraries built without TSAN, while silencing issues that might be present within OpenMPI itself. In this case, the error is coming from mpiexec, which exists as a completely separate process from my application. So, I am aware that this is probably an error in mpiexec, but would like to silence it anyway, and I can't seem to do that. In other words, this isn't a problem with TSAN yielding a false positive, but yielding an error with no way for me to silence it. Kyle ___ cfe-users mailing list cfe-users@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] clang-tidy
Le mer. 5 févr. 2020 09 h 48, Chris Green 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::stringstd::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-us...@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=
>
>
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
