[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:29
+void OwningMemoryCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11) {
+    return;
----------------
Can elide the braces.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:32
+  }
+  const auto ownerDecl = typeAliasTemplateDecl(hasName("::gsl::owner"));
+  const auto isOwnerType = hasType(ownerDecl);
----------------
Variable names should start with an uppercase letter (here and elsewhere).


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:39
+
+  // Find delete expressions, that delete non owners.
+  Finder->addMatcher(
----------------
Can remove the comma and add a hyphen to non-owners.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:55
+
+  // Matching initialization of owners with non owners, nor creating owners.
+  Finder->addMatcher(
----------------
non-owners


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:75
+
+  // Matching on assignment operations, where the RHS is a newly created owner,
+  // but the LHS is not an owner.
----------------
Can remove the first comma.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:83
+
+  // Matching on initialization operations, where the initial value is a newly
+  // created owner, but the LHS is not an owner.
----------------
Can remove the first comma.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:93
+
+  // Match on all function calls, that expect owners as arguments, but didn't
+  // get them.
----------------
Can remove the first comma.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:101
+
+  // Matching for function calls, where one argument is a created owner, but the
+  // parameter type is not an owner.
----------------
Can remove the first comma.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:108
+
+  // Matching on functions, that return an owner/resource, but don't declare
+  // their return type as owner.
----------------
Can remove the first comma.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:117
+
+  // Match on classes, that have an owner as member, but don't declare a
+  // destructor to properly release the owner.
----------------
Can remove the first comma.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:128
+
+void OwningMemoryCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto &Nodes = Result.Nodes;
----------------
Please split this function up into smaller functions that check only the individual components.


================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.h:19
+
+/// Checks for common usecases for gsl::owner and enforces the unique owner nature
+/// of it whenever possible.
----------------
usecases -> use cases


https://reviews.llvm.org/D36354