[PATCH] D17811: [clang-tidy] Add check to detect dangling references in value handlers.

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.

================
Comment at: clang-tidy/misc/DanglingHandleCheck.cpp:24
@@ +23,3 @@
+
+std::vector<std::string> parseClasses(StringRef Option) {
+  SmallVector<StringRef, 4> Classes;
----------------
alexfh wrote:
> A very similar code has been added recently to clang-tidy/utils/HeaderFileExtensionsUtils.*. Maybe make that code generic enough and reuse it?
It is not the same.
That one is filtering the characters using isAlphanumeric.
Type names can have non-alpha chars as part of template instantiations.

On the other hand, it is a copy of the one from FasterStringFindCheck.
We could move it to a central location.

================
Comment at: clang-tidy/misc/DanglingHandleCheck.cpp:57
@@ +56,3 @@
+ast_matchers::internal::Matcher<RecordDecl> isASequence() {
+  return hasAnyName("::std::deque", "::std::forward_list", "::std::list",
+                    "::std::vector");
----------------
jbcoe wrote:
> Will this (and similar checkers) handle inline namespaces?
Yes. hasName() and hasAnyName() have been fixed to look through inline namespaces.

================
Comment at: clang-tidy/misc/DanglingHandleCheck.cpp:116
@@ +115,3 @@
+  return cxxRecordDecl(internal::Matcher<NamedDecl>(
+                           new internal::HasNameMatcher(HandleClasses)))
+      .bind("handle");
----------------
alexfh wrote:
> This use of internal classes doesn't look nice. Can you add a `hasAnyName` overload taking a `set` or an `ArrayRef` of names (or maybe a template version that'll take a range of something convertible to StringRef)?
Will do.

================
Comment at: test/clang-tidy/misc-dangling-handle.cpp:73
@@ +72,3 @@
+  StringRef(const char*);  // NOLINT
+  StringRef(const std::string&);  // NOLINT
+};
----------------
jbcoe wrote:
> What does `NOLINT` do here?
Removed. Not needed here.

================
Comment at: test/clang-tidy/misc-dangling-handle.cpp:85
@@ +84,3 @@
+  std::string_view view_2 = ReturnsAString();
+  // CHECK-MESSAGES: [[@LINE-1]]:29: warning: std::basic_string_view outlives
+
----------------
jbcoe wrote:
> This (and other) check-messages line looks truncated.
They are truncated on purpose.
We use the full message in the first one to check the message.
The rest are truncated to fit in 80 chars.


http://reviews.llvm.org/D17811