Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
aaron.ballman updated this revision to Diff 32697.
aaron.ballman added a comment.
In http://reviews.llvm.org/D11784#228764, @alexfh wrote:
In http://reviews.llvm.org/D11784#227939, @aaron.ballman wrote:
Addressed review comments. I re-ran the updated patch against LLVM and
Clang, and there were some more false positives that I would like to
address if possible. It seems my previous run against the source base was
before expanding the scope of the patch to include more than just base
class initialization (sorry for the earlier misinformation).
1. SourceMgr.h:58 is an example where the checker issues a diagnostic (for
IncludeLoc), but given the triviality of the type, I don't see a reason to
diagnose. However, this requires support from Sema, so I think a FIXME may
be the best I can do. Note, adding a call to std::move() in these instances
is not wrong, it's just not particularly useful.
Determining whether a type is trivially-copyable can be done on the AST level
without Sema (see QualType::isTriviallyCopyableType).
That should get us closer, but doesn't quite cut it. I think I may be able to
combine this with CXXRecordDecl::isTriviallyCopyable() to get us close enough
to cut down on the number of false positives. Thank you for pointing this out,
it is implemented in this patch.
2. we should not be warning on anything an implicit constructor does. For
instance LiveQueryResult is triggering this because of SlotIndex. This
should be fixed with this patch.
Running over Clang and LLVM, there are 7 distinct false positives
(repeated due to being in header files) and they all relate to triviality.
The total false positive count was 832 of which two warnings (SourceMgr.h
and Preprocessor.h) accounted for probably close to 90% of the diagnostics.
This time around there were no true positives.
Can you list the list of unique locations?
With the triviality implementation, I now get zero false positives (and zero
true positives) in the Clang and LLVM source base.
http://reviews.llvm.org/D11784
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/MoveConstructorInitCheck.cpp
clang-tidy/misc/MoveConstructorInitCheck.h
test/clang-tidy/misc-move-constructor-init.cpp
Index: test/clang-tidy/misc-move-constructor-init.cpp
===
--- test/clang-tidy/misc-move-constructor-init.cpp
+++ test/clang-tidy/misc-move-constructor-init.cpp
@@ -0,0 +1,78 @@
+// RUN: clang-tidy %s -checks=-*,misc-move-constructor-init -- -std=c++14 | FileCheck %s -implicit-check-not={{warning|error}}:
+
+template class T struct remove_reference {typedef T type;};
+template class T struct remove_referenceT {typedef T type;};
+template class T struct remove_referenceT {typedef T type;};
+
+template typename T
+typename remove_referenceT::type move(T arg) {
+ return static_casttypename remove_referenceT::type(arg);
+}
+
+struct C {
+ C() = default;
+ C(const C) = default;
+};
+
+struct B {
+ B() {}
+ B(const B) {}
+ B(B ) {}
+};
+
+struct D : B {
+ D() : B() {}
+ D(const D RHS) : B(RHS) {}
+ // CHECK: :[[@LINE+3]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-init]
+ // CHECK: 19:3: note: copy constructor being called
+ // CHECK: 20:3: note: candidate move constructor here
+ D(D RHS) : B(RHS) {}
+};
+
+struct E : B {
+ E() : B() {}
+ E(const E RHS) : B(RHS) {}
+ E(E RHS) : B(move(RHS)) {} // ok
+};
+
+struct F {
+ C M;
+
+ F(F ) : M(C()) {} // ok
+};
+
+struct G {
+ G() = default;
+ G(const G) = default;
+ G(G) = delete;
+};
+
+struct H : G {
+ H() = default;
+ H(const H) = default;
+ H(H RHS) : G(RHS) {} // ok
+};
+
+struct I {
+ I(const I ) = default; // suppresses move constructor creation
+};
+
+struct J : I {
+ J(J RHS) : I(RHS) {} // ok
+};
+
+struct K {}; // Has implicit copy and move constructors, is trivially copyable
+struct L : K {
+ L(L RHS) : K(RHS) {} // ok
+};
+
+struct M {
+ B Mem;
+ // CHECK: :[[@LINE+1]]:16: warning: move constructor initializes class member by calling a copy constructor [misc-move-constructor-init]
+ M(M RHS) : Mem(RHS.Mem) {}
+};
+
+struct N {
+ B Mem;
+ N(N RHS) : Mem(move(RHS.Mem)) {}
+};
Index: clang-tidy/misc/MoveConstructorInitCheck.h
===
--- clang-tidy/misc/MoveConstructorInitCheck.h
+++ clang-tidy/misc/MoveConstructorInitCheck.h
@@ -0,0 +1,32 @@
+//===--- MoveConstructorInitCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. With the triviality implementation, I now get zero false positives (and zero true positives) in the Clang and LLVM source base. Awesome! Thanks for working on this! LGTM http://reviews.llvm.org/D11784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
alexfh added a comment.
Sorry for the long delay.
Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:40
@@ +39,3 @@
+ for (const auto *Ctor : CopyCtor-getParent()-ctors()) {
+if (Ctor-isMoveConstructor()
+Ctor-getAccess() = AS_protected
clang-format?
Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:46
@@ +45,3 @@
+ //
+ // FIXME: Determine whether the move constructor is a viable candidate
+ // for the ctor-initializer, perhaps provide a fixit that suggests
This seems to be rather important to do from the beginning. Otherwise the check
may be too noisy. BTW, did you run it over LLVM and Clang sources? Would be
useful for some smoke testing.
Comment at: test/clang-tidy/misc-move-constructor-init.cpp:64
@@ +63,3 @@
+struct J : I {
+ // CHECK-NOT: warning:
+ J(J RHS) : I(RHS) {} // ok
I'd suggest using FileCheck -implicit-check-not='{{warning|error}}:' instead of
stuffing the code with `// CHECK-NOT: warning:`. It will make the test more
consistent with the other tests that use the clang_tidy_test.sh script.
http://reviews.llvm.org/D11784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
aaron.ballman updated this revision to Diff 32559.
aaron.ballman added a comment.
Addressed review comments. I re-ran the updated patch against LLVM and Clang,
and there were some more false positives that I would like to address if
possible. It seems my previous run against the source base was before expanding
the scope of the patch to include more than just base class initialization
(sorry for the earlier misinformation).
1. SourceMgr.h:58 is an example where the checker issues a diagnostic (for
IncludeLoc), but given the triviality of the type, I don't see a reason to
diagnose. However, this requires support from Sema, so I think a FIXME may be
the best I can do. Note, adding a call to std::move() in these instances is not
wrong, it's just not particularly useful.
2. we should not be warning on anything an implicit constructor does. For
instance LiveQueryResult is triggering this because of SlotIndex. This should
be fixed with this patch.
Running over Clang and LLVM, there are 7 distinct false positives (repeated due
to being in header files) and they all relate to triviality. The total false
positive count was 832 of which two warnings (SourceMgr.h and Preprocessor.h)
accounted for probably close to 90% of the diagnostics. This time around there
were no true positives.
http://reviews.llvm.org/D11784
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/MoveConstructorInitCheck.cpp
clang-tidy/misc/MoveConstructorInitCheck.h
test/clang-tidy/misc-move-constructor-init.cpp
Index: test/clang-tidy/misc-move-constructor-init.cpp
===
--- test/clang-tidy/misc-move-constructor-init.cpp
+++ test/clang-tidy/misc-move-constructor-init.cpp
@@ -0,0 +1,79 @@
+// RUN: clang-tidy %s -checks=-*,misc-move-constructor-init -- -std=c++14 | FileCheck %s -implicit-check-not={{warning|error}}:
+
+template class T struct remove_reference {typedef T type;};
+template class T struct remove_referenceT {typedef T type;};
+template class T struct remove_referenceT {typedef T type;};
+
+template typename T
+typename remove_referenceT::type move(T arg) {
+ return static_casttypename remove_referenceT::type(arg);
+}
+
+struct C {
+ C() = default;
+ C(const C) = default;
+};
+
+struct B {
+ B() {}
+ B(const B) {}
+ B(B ) {}
+};
+
+struct D : B {
+ D() : B() {}
+ D(const D RHS) : B(RHS) {}
+ // CHECK: :[[@LINE+3]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-init]
+ // CHECK: 19:3: note: copy constructor being called
+ // CHECK: 20:3: note: candidate move constructor here
+ D(D RHS) : B(RHS) {}
+};
+
+struct E : B {
+ E() : B() {}
+ E(const E RHS) : B(RHS) {}
+ E(E RHS) : B(move(RHS)) {} // ok
+};
+
+struct F {
+ C M;
+
+ F(F ) : M(C()) {} // ok
+};
+
+struct G {
+ G() = default;
+ G(const G) = default;
+ G(G) = delete;
+};
+
+struct H : G {
+ H() = default;
+ H(const H) = default;
+ H(H RHS) : G(RHS) {} // ok
+};
+
+struct I {
+ I(const I ) = default; // suppresses move constructor creation
+};
+
+struct J : I {
+ J(J RHS) : I(RHS) {} // ok
+};
+
+struct K {}; // Has implicit copy and move constructors
+struct L : K {
+ // CHECK: :[[@LINE+1]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-init]
+ L(L RHS) : K(RHS) {}
+};
+
+struct M {
+ B Mem;
+ // CHECK: :[[@LINE+1]]:16: warning: move constructor initializes class member by calling a copy constructor [misc-move-constructor-init]
+ M(M RHS) : Mem(RHS.Mem) {}
+};
+
+struct N {
+ B Mem;
+ N(N RHS) : Mem(move(RHS.Mem)) {}
+};
Index: clang-tidy/misc/MoveConstructorInitCheck.h
===
--- clang-tidy/misc/MoveConstructorInitCheck.h
+++ clang-tidy/misc/MoveConstructorInitCheck.h
@@ -0,0 +1,32 @@
+//===--- MoveConstructorInitCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H
+
+#include ../ClangTidy.h
+
+namespace clang {
+namespace tidy {
+
+/// \brief The check flags user-defined move constructors that have a
+/// ctor-initializer initializing a member or base class through a copy
+/// constructor instead of a move constructor.
+class MoveConstructorInitCheck : public ClangTidyCheck {
+public:
+ MoveConstructorInitCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
aaron.ballman marked 5 inline comments as done. aaron.ballman added a comment. http://reviews.llvm.org/D11784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:40
@@ +39,3 @@
+ for (const auto *Ctor : CopyCtor-getParent()-ctors()) {
+if (Ctor-isMoveConstructor()
+Ctor-getAccess() = AS_protected
alexfh wrote:
clang-format?
I thought the current formatting was an improvement over what clang-format
produces (for those of us with debuggers that aren't as good at subexpression
highlighting). I'm fine either way, though.
Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:46
@@ +45,3 @@
+ //
+ // FIXME: Determine whether the move constructor is a viable candidate
+ // for the ctor-initializer, perhaps provide a fixit that suggests
alexfh wrote:
This seems to be rather important to do from the beginning. Otherwise the
check may be too noisy. BTW, did you run it over LLVM and Clang sources?
Would be useful for some smoke testing.
In order to do that, I would need access to more parts of Sema. The check, as
it currently stands, is fairly reasonable from what I can tell. The false
positive rate appears to be low. I ran it over Clang and LLVM and it did point
out one debatably-true-positive (which we've since resolved) with no
false-positives. In testing other code bases, the diagnostic was not chatty,
but perhaps they did not make heavy use of move semantics.
Comment at: test/clang-tidy/misc-move-constructor-init.cpp:64
@@ +63,3 @@
+struct J : I {
+ // CHECK-NOT: warning:
+ J(J RHS) : I(RHS) {} // ok
alexfh wrote:
I'd suggest using FileCheck -implicit-check-not='{{warning|error}}:' instead
of stuffing the code with `// CHECK-NOT: warning:`. It will make the test
more consistent with the other tests that use the clang_tidy_test.sh script.
Can do (though I am explicitly not using clang_tidy_test.sh because I am
working on Windows and all those tests are currently disabled due to REQUIRES:
shell :-()
http://reviews.llvm.org/D11784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
aaron.ballman updated this revision to Diff 32175.
aaron.ballman added a comment.
Updated the patch to handle member initializers as well as base class
initializers. This changes the name of things from base to init as well.
http://reviews.llvm.org/D11784
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/MoveConstructorInitCheck.cpp
clang-tidy/misc/MoveConstructorInitCheck.h
test/clang-tidy/misc-move-constructor-init.cpp
Index: test/clang-tidy/misc-move-constructor-init.cpp
===
--- test/clang-tidy/misc-move-constructor-init.cpp
+++ test/clang-tidy/misc-move-constructor-init.cpp
@@ -0,0 +1,83 @@
+// RUN: clang-tidy %s -checks=-*,misc-move-constructor-init -- -std=c++14 | FileCheck %s
+
+template class T struct remove_reference {typedef T type;};
+template class T struct remove_referenceT {typedef T type;};
+template class T struct remove_referenceT {typedef T type;};
+
+template typename T
+typename remove_referenceT::type move(T arg) {
+ return static_casttypename remove_referenceT::type(arg);
+}
+
+struct C {
+ C() = default;
+ C(const C) = default;
+};
+
+struct B {
+ B() {}
+ B(const B) {}
+ B(B ) {}
+};
+
+struct D : B {
+ D() : B() {}
+ D(const D RHS) : B(RHS) {}
+ // CHECK: :[[@LINE+3]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-init]
+ // CHECK: 19:3: note: copy constructor being called
+ // CHECK: 20:3: note: candidate move constructor here
+ D(D RHS) : B(RHS) {}
+};
+
+struct E : B {
+ E() : B() {}
+ E(const E RHS) : B(RHS) {}
+ // CHECK-NOT: warning:
+ E(E RHS) : B(move(RHS)) {} // ok
+};
+
+struct F {
+ C M;
+
+ // CHECK-NOT: warning:
+ F(F ) : M(C()) {}
+};
+
+struct G {
+ G() = default;
+ G(const G) = default;
+ G(G) = delete;
+};
+
+struct H : G {
+ H() = default;
+ H(const H) = default;
+ // CHECK-NOT: warning:
+ H(H RHS) : G(RHS) {} // ok
+};
+
+struct I {
+ I(const I ) = default; // suppresses move constructor creation
+};
+
+struct J : I {
+ // CHECK-NOT: warning:
+ J(J RHS) : I(RHS) {} // ok
+};
+
+struct K {}; // Has implicit copy and move constructors
+struct L : K {
+ // CHECK: :[[@LINE+1]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-init]
+ L(L RHS) : K(RHS) {}
+};
+
+struct M {
+ B Mem;
+ // CHECK: :[[@LINE+1]]:16: warning: move constructor initializes class member by calling a copy constructor [misc-move-constructor-init]
+ M(M RHS) : Mem(RHS.Mem) {}
+};
+
+struct N {
+ B Mem;
+ N(N RHS) : Mem(move(RHS.Mem)) {}
+};
Index: clang-tidy/misc/MoveConstructorInitCheck.h
===
--- clang-tidy/misc/MoveConstructorInitCheck.h
+++ clang-tidy/misc/MoveConstructorInitCheck.h
@@ -0,0 +1,32 @@
+//===--- MoveConstructorInitCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H
+
+#include ../ClangTidy.h
+
+namespace clang {
+namespace tidy {
+
+/// \brief The check flags user-defined move constructors that have a
+/// ctor-initializer initializing a member or base class through a copy
+/// constructor instead of a move constructor.
+class MoveConstructorInitCheck : public ClangTidyCheck {
+public:
+ MoveConstructorInitCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H
Index: clang-tidy/misc/MoveConstructorInitCheck.cpp
===
--- clang-tidy/misc/MoveConstructorInitCheck.cpp
+++ clang-tidy/misc/MoveConstructorInitCheck.cpp
@@ -0,0 +1,69 @@
+//===--- MoveConstructorInitCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include MoveConstructorInitCheck.h
+#include clang/AST/ASTContext.h
+#include clang/ASTMatchers/ASTMatchFinder.h
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void MoveConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
+ Finder-addMatcher(
+
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
aaron.ballman added a comment.
In http://reviews.llvm.org/D11784#224430, @alexfh wrote:
In http://reviews.llvm.org/D11784#224421, @aaron.ballman wrote:
In http://reviews.llvm.org/D11784#224386, @alexfh wrote:
One thing I am not certain of in this patch is how to test it. I have
some rudimentary tests, but am unable to test the note: diagnostics
from
FileCheck (attempting to add any cause the warning: diagnostics to
not be found).
Can you give an example of what you do and what results do you get?
I put CHECK: :[[@LINE+1]]:3: note: copy constructor being called into the
source file, and tests no longer pass because it cannot find the matching
warning: diagnostic. If I then remove the warning diagnostic, the tests
pass again. So it seems I can test one or the other, but not both.
Specifically (with note and warning):
Might it be that you got the line offsets in @LINE incorrectly? A test like
this should work, if both the warning and the note are on the same line:
// CHECK: :[[@LINE+2]]:...: warning:
// CHECK: :[[@LINE+1]]:...: note:
some_line_that_generates_a_warning_with_a_note();
The warning is on line 28, the note is on line 20.
If your note is generated on a different line than the warning (e.g. class
declaration vs. the incorrect use of a variable), then you may have to use
@LINE+x for the warning and the line number verbatim for the note check.
Ah, interesting -- I was avoiding that because of how fragile it is. Also, it
seems to require me grouping the CHECK lines together. e.g.) this fails:
struct B {
B() {}
B(const B) {}
B(B ) {} // CHECK: 19:3: note: copy constructor being called
};
struct D : B {
D() : B() {}
D(const D RHS) : B(RHS) {}
// CHECK: :[[@LINE+1]]:16: warning: move constructor initializes base class
by calling a copy constructor [misc-move-constructor-base]
D(D RHS) : B(RHS) {}
};
But this succeeds:
struct B {
B() {}
B(const B) {}
B(B ) {}
};
struct D : B {
D() : B() {}
D(const D RHS) : B(RHS) {}
// CHECK: :[[@LINE+2]]:16: warning: move constructor initializes base class
by calling a copy constructor [misc-move-constructor-base]
// CHECK: 19:3: note: copy constructor being called
D(D RHS) : B(RHS) {}
};
In any event, there's now a solution that lets me test the notes, so that's
great. Thank you!
~Aaron
http://reviews.llvm.org/D11784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
alexfh added a comment. In http://reviews.llvm.org/D11784#224433, @aaron.ballman wrote: In http://reviews.llvm.org/D11784#224430, @alexfh wrote: In http://reviews.llvm.org/D11784#224421, @aaron.ballman wrote: In http://reviews.llvm.org/D11784#224386, @alexfh wrote: Ah, interesting -- I was avoiding that because of how fragile it is. Also, it seems to require me grouping the CHECK lines together. e.g.) this fails: FileCheck is rather primitive. It's behavior may confuse at first, but once you get accustomed to it, everything becomes clear. http://reviews.llvm.org/D11784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
alexfh added a comment. In http://reviews.llvm.org/D11784#224421, @aaron.ballman wrote: In http://reviews.llvm.org/D11784#224386, @alexfh wrote: One thing I am not certain of in this patch is how to test it. I have some rudimentary tests, but am unable to test the note: diagnostics from FileCheck (attempting to add any cause the warning: diagnostics to not be found). Can you give an example of what you do and what results do you get? I put CHECK: :[[@LINE+1]]:3: note: copy constructor being called into the source file, and tests no longer pass because it cannot find the matching warning: diagnostic. If I then remove the warning diagnostic, the tests pass again. So it seems I can test one or the other, but not both. Specifically (with note and warning): Might it be that you got the line offsets in @LINE incorrectly? A test like this should work, if both the warning and the note are on the same line: // CHECK: :[[@LINE+2]]:...: warning: // CHECK: :[[@LINE+1]]:...: note: some_line_that_generates_a_warning_with_a_note(); If your note is generated on a different line than the warning (e.g. class declaration vs. the incorrect use of a variable), then you may have to use @LINE+x for the warning and the line number verbatim for the note check. http://reviews.llvm.org/D11784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
alexfh added a comment. In http://reviews.llvm.org/D11784#220654, @aaron.ballman wrote: Ping? Sorry for the delay. I'm on vacation currently. Will be back tomorrow. - Alex http://reviews.llvm.org/D11784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11784: [PATCH] clang-tidy check for incorrect move constructor initializers
aaron.ballman added a comment. Ping? FWIW, this patch almost caught a bug in LLVM. ;-) DependenceAnalysis.h has a class: FullDependence which would suffer from this problem if the Dependence base class did not accidentally suppress creation of the move constructor by defaulting only the copy constructor. (Separate patch forthcoming.) I think that may be a reasonable option for this patch to test for, but wasn't quite certain. What do others think? ~Aaron http://reviews.llvm.org/D11784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
