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
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
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
==
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
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-
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
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.
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
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|
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
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
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
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
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
_
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
>
15 matches
Mail list logo