[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-07 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. LGTM, leave this to alexfh's approval. Repository: rL LLVM https://reviews.llvm.org/D34002 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from one minor nit, LGTM Comment at: clang-tidy/misc/NoexceptMoveConstructorCheck.cpp:23 // provide any benefit to other languages, despite being benig

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-07 Thread Yan Wang via Phabricator via cfe-commits
yawanng updated this revision to Diff 101834. yawanng marked an inline comment as done. https://reviews.llvm.org/D34002 Files: clang-tidy/misc/NoexceptMoveConstructorCheck.cpp test/clang-tidy/misc-noexcept-move-constructor.cpp Index: test/clang-tidy/misc-noexcept-move-constructor.cpp ==

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. IIUC, when `vector` (for a class `T` that has both move and copy constructors) resizes, it will prefer move constructors, but only if they're declared `noexcept`. This is true even if `-fno-exceptions` is on. So I don't think this check should depend on `-fno-exceptions

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Here's a small test: https://godbolt.org/g/nNZgUb (look for `T::T(` and `S::S(` calls, adding `-fno-exceptions` doesn't change which constructors are called). https://reviews.llvm.org/D34002 ___ cfe-commits mailing list cfe-

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-08 Thread Phil Camp via Phabricator via cfe-commits
FlameTop added inline comments. Comment at: test/clang-tidy/misc-noexcept-move-constructor.cpp:2 +// RUN: clang-tidy %s -checks="-*,misc-noexcept-move-constructor" -- -std=c++11 \ +// RUN: | FileCheck %s -check-prefix=CHECK-EXCEPTIONS \ +// RUN: -implicit-check-not="{{warnin

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-08 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. In https://reviews.llvm.org/D34002#775830, @alexfh wrote: > IIUC, when `vector` (for a class `T` that has both move and copy > constructors) resizes, it will prefer move constructors, but only if they're > declared `noexcept`. This is true even if `-fno-exceptions` is on.

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D34002#776193, @chh wrote: > In https://reviews.llvm.org/D34002#775830, @alexfh wrote: > > > IIUC, when `vector` (for a class `T` that has both move and copy > > constructors) resizes, it will prefer move constructors, but only if > > they're

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-08 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: test/clang-tidy/misc-noexcept-move-constructor.cpp:2 +// RUN: clang-tidy %s -checks="-*,misc-noexcept-move-constructor" -- -std=c++11 \ +// RUN: | FileCheck %s -check-prefix=CHECK-EXCEPTIONS \ +// RUN: -implicit-check-not="{{warning|

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-08 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. Android source is suppressing misc-noexcept-move-constructor warnings because -fno-exceptions is used and Android does not like to add more exception specific code. It's not a big deal for Android to suppress this check one way or the other. I don't mind reverting it, just cu

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D34002#776551, @chh wrote: > Android source is suppressing misc-noexcept-move-constructor warnings > because -fno-exceptions is used and Android does not like to add more > exception specific code. As I've said, the lack of `noexcept` on mov

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-09 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. Thanks for providing the ODR reason for Android -fno-exceptions users to change source code or suppress this warning themselves. https://reviews.llvm.org/D34002 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-09 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D34002#776948, @chh wrote: > Thanks for providing the ODR reason for Android -fno-exceptions users to > change source code or suppress this warning themselves. I'm not sure I understand your comment. Could you clarify? Here's an explanation

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-09 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. I understood the mentioned ODR issue, which is useful for users to understand the compiler choice. For -fno-exceptions users, they should change source code to work with mixed cases, or just ignore mixed cases and suppress this warning. https://reviews.llvm.org/D34002 _

Re: [PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

2017-06-08 Thread Aaron Ballman via cfe-commits
On Thu, Jun 8, 2017 at 6:24 PM, Alexander Kornienko via Phabricator wrote: > alexfh added a comment. > > In https://reviews.llvm.org/D34002#776193, @chh wrote: > >> In https://reviews.llvm.org/D34002#775830, @alexfh wrote: >> >> > IIUC, when `vector` (for a class `T` that has both move and copy >