[PATCH] D36354: [clang-tidy] Implement type-based check for `gsl::owner`
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 25 09:26:29 PDT 2017
aaron.ballman added inline comments.
================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:202
+
+ // Initializer of class constructors, that initialize owners.
+ if (OwnerInitializer) {
----------------
Can remove the comma.
================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:239
+ diag(BadOwnerInitialization->getLocStart(),
+ "initializing non owner %0 with a newly created 'gsl::owner<>'")
+ << BadOwnerInitialization->getType()
----------------
s/non owner/non-owner
================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:256
+ assert(BadOwnerParameter && "parameter for the problematic argument not found");
+ diag(BadOwnerArgument->getLocStart(), "initializing non owner argument of "
+ "type %0 with a newly created "
----------------
s/non owner/non-owner
================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:292
+ assert(DeclaredOwnerMember &&
+ "Match on class with bad destructor, but without an declared owner");
+
----------------
s/an declared/a declared
Also, you should either add punctuation or remove the capital M in Match.
================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:294
+
+ diag(BadClass->getLocStart(),
+ "class with an 'gsl::owner<>' as member but without declared "
----------------
Instead of diagnosing this on the class, why not diagnose the individual offending members (and then you don't need a note, either)? (Note, that may change the suggested diagnostic wording below).
================
Comment at: clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp:295
+ diag(BadClass->getLocStart(),
+ "class with an 'gsl::owner<>' as member but without declared "
+ "destructor to release the owner");
----------------
This diagnostic isn't grammatically correct; how about: `class with a member variable of type 'gsl::owner<>' but no destructor to release the owned resource`?
https://reviews.llvm.org/D36354
More information about the cfe-commits
mailing list