[clang-tidy] Emit note for variable declaration that are later deleted

[JonasToth]

Summary

This patch introduces a note for variable declaration that are later deleted.
Adds FIXME notes for possible automatic type-rewriting positions as well.

[aaron.ballman]

clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp

160

Spurious comma in the comment.

162

Do not use auto here.

[JonasToth]

• adress aarons nits

[JonasToth]

• fixed review comments

[aaron.ballman]

LGTM!

[JonasToth]

merge master

[JonasToth]

committed in rL314913

clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp

bool OwningMemoryCheck::handleDeletion(const BoundNodes &Nodes) {
 
   // Deletion of non-owners, with `delete variable;`
   if (DeleteStmt) {
     diag(DeleteStmt->getLocStart(),
          "deleting a pointer through a type that is "
          "not marked 'gsl::owner<>'; consider using a "
          "smart pointer instead")
         << DeletedVariable->getSourceRange();
+
+    // FIXME: The declaration of the variable that was deleted can be
+    // rewritten.
+    const ValueDecl *Decl = DeletedVariable->getDecl();
+    diag(Decl->getLocStart(), "variable declared here", DiagnosticIDs::Note)
+        << Decl->getSourceRange();
+
     return true;
   }
   return false;
 }
 
 bool OwningMemoryCheck::handleExpectedOwner(const BoundNodes &Nodes) {
   // Result of function call matchers.
   const auto *ExpectedOwner = Nodes.getNodeAs<Expr>("expected_owner_argument");
bool OwningMemoryCheck::handleAssignmentFromNewOwner(const BoundNodes &Nodes) {
   }
 
   // Bad initialization of non-owners, where the RHS is a newly created owner.
   if (BadOwnerInitialization) {
     diag(BadOwnerInitialization->getLocStart(),
          "initializing non-owner %0 with a newly created 'gsl::owner<>'")
         << BadOwnerInitialization->getType()
         << BadOwnerInitialization->getSourceRange();
-    // FIXME: FixitHint to rewrite the type if possible.
+
+    // FIXME: FixitHint to rewrite the type of the initialized variable
+    // as 'gsl::owner<OriginalType>'
 
     // If the type of the variable was deduced, the wrapping owner typedef is
     // eliminated, therefore the check emits a special note for that case.
     if (Nodes.getNodeAs<AutoType>("deduced_type")) {
       diag(BadOwnerInitialization->getLocStart(),
            "type deduction did not result in an owner", DiagnosticIDs::Note);
     }
     return true;

 bool OwningMemoryCheck::handleReturnValues(const BoundNodes &Nodes) {
   // Function return statements, that are owners/resources, but the function
   // declaration does not declare its return value as owner.
   const auto *BadReturnType = Nodes.getNodeAs<ReturnStmt>("bad_owner_return");
   const auto *Function = Nodes.getNodeAs<FunctionDecl>("function_decl");
 
   // Function return values, that should be owners but aren't.
   if (BadReturnType) {
-    // The returned value is of type owner, but not the declared return type.
+    // The returned value is a resource or variable that was not annotated with
+    // owner<> and the function return type is not owner<>.
     diag(BadReturnType->getLocStart(),
          "returning a newly created resource of "
          "type %0 or 'gsl::owner<>' from a "
          "function whose return type is not 'gsl::owner<>'")
         << Function->getReturnType() << BadReturnType->getSourceRange();
-    // The returned value is a resource that was not annotated with owner<> and
-    // the function return type is not owner<>.
+
+    // FIXME: Rewrite the return type as 'gsl::owner<OriginalType>'
     return true;
   }
   return false;
 }
 
 bool OwningMemoryCheck::handleOwnerMembers(const BoundNodes &Nodes) {
   // Classes, that have owners as member, but do not declare destructors
   // accordingly.

test/clang-tidy/cppcoreguidelines-owning-memory.cpp

void test_deletion() {
 
   gsl::owner<int *> owned_int2 = new int[42];
   delete[] owned_int2; // Good
 
   int *unowned_int1 = new int(42); // BAD, since new creates and owner
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
   delete unowned_int1; // BAD, since no owner
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead
+  // CHECK-MESSAGES: [[@LINE-4]]:3: note: variable declared here
 
   int *unowned_int2 = new int[42]; // BAD, since new creates and owner
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
   delete[] unowned_int2; // BAD since no owner
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead
+  // CHECK-MESSAGES: [[@LINE-4]]:3: note: variable declared here
 
   delete new int(42);   // Technically ok, but stupid
   delete[] new int[42]; // Technically ok, but stupid
 }
 
 void test_owner_function_calls() {
   int stack_int = 42;
   int *unowned_int1 = &stack_int;