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

This check implements the typebased semantic of `gsl::owner`.
Meaning, that 
- only `gsl::owner` is allowed to get `delete`d
- `new` expression must be assigned to `gsl::owner`
- function calls that expect `gsl::owner` as argument, must get either an owner
  or a newly created and recognized resource (in the moment only `new`ed memory)
- assignment to `gsl::owner` must be either a resource or another owner
- functions returning an `gsl::owner` are considered as factories, and their result
  must be assigned to an `gsl::owner`
- classes that have an `gsl::owner`-member must declare a non-default destructor

There are some problems that occur when typededuction is in place.
For example `auto Var = function_that_returns_owner();` the type of `Var` will not be
an `gsl::owner`. This case is catched, and explicitly noted.

But cases like fully templated functions
```
template <typename T> 
void f(T t) { delete t; }
// ...
f(gsl::owner<int*>(new int(42)));
```
Will created false positive (the deletion is problematic), since the type deduction
removes the wrapping `typeAlias`.

Please give your comments :)

llvm-svn: 313043

clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

@@ -4,6 +4,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
   CppCoreGuidelinesTidyModule.cpp
   InterfacesGlobalInitCheck.cpp
   NoMallocCheck.cpp
+  OwningMemoryCheck.cpp
   ProBoundsArrayToPointerDecayCheck.cpp
   ProBoundsConstantArrayIndexCheck.cpp
   ProBoundsPointerArithmeticCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

@@ -13,6 +13,7 @@
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "NoMallocCheck.h"
+#include "OwningMemoryCheck.h"
 #include "ProBoundsArrayToPointerDecayCheck.h"
 #include "ProBoundsConstantArrayIndexCheck.h"
 #include "ProBoundsPointerArithmeticCheck.h"
@@ -37,6 +38,8 @@ public:
     CheckFactories.registerCheck<InterfacesGlobalInitCheck>(
         "cppcoreguidelines-interfaces-global-init");
     CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc");
+    CheckFactories.registerCheck<OwningMemoryCheck>(
+        "cppcoreguidelines-owning-memory");
     CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
         "cppcoreguidelines-pro-bounds-array-to-pointer-decay");
     CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(

clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp

@@ -0,0 +1,315 @@
+//===--- OwningMemoryCheck.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 "OwningMemoryCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <string>
+#include <vector>
+
+using namespace clang::ast_matchers;
+using namespace clang::ast_matchers::internal;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Match common cases, where the owner semantic is relevant, like function
+/// calls, delete expressions and others.
+void OwningMemoryCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  const auto OwnerDecl = typeAliasTemplateDecl(hasName("::gsl::owner"));
+  const auto IsOwnerType = hasType(OwnerDecl);
+  const auto CreatesOwner =
+      anyOf(cxxNewExpr(), callExpr(callee(functionDecl(
+                              returns(qualType(hasDeclaration(OwnerDecl)))))));
+  const auto ConsideredOwner = anyOf(IsOwnerType, CreatesOwner);
+
+  // Find delete expressions that delete non-owners.
+  Finder->addMatcher(
+      cxxDeleteExpr(
+          hasDescendant(
+              declRefExpr(unless(ConsideredOwner)).bind("deleted_variable")))
+          .bind("delete_expr"),
+      this);
+
+  // Matching assignment to owners, with the rhs not being an owner nor creating
+  // one.
+  Finder->addMatcher(binaryOperator(allOf(matchers::isAssignmentOperator(),
+                                          hasLHS(IsOwnerType),
+                                          hasRHS(unless(ConsideredOwner))))
+                         .bind("owner_assignment"),
+                     this);
+
+  // Matching initialization of owners with non-owners, nor creating owners.
+  Finder->addMatcher(
+      namedDecl(
+          varDecl(allOf(hasInitializer(unless(ConsideredOwner)), IsOwnerType))
+              .bind("owner_initialization")),
+      this);
+
+  // Match class member initialization that expects owners, but does not get
+  // them.
+  Finder->addMatcher(
+      cxxRecordDecl(has(cxxConstructorDecl(forEachConstructorInitializer(
+          cxxCtorInitializer(
+              allOf(
+                  isMemberInitializer(), forField(IsOwnerType),
+                  withInitializer(
+                      // Avoid templatesdeclaration with excluding parenListExpr.
+                      allOf(unless(ConsideredOwner), unless(parenListExpr())))))
+              .bind("owner_member_initializer"))))),
+      this);
+
+  // Matching on assignment operations where the RHS is a newly created owner,
+  // but the LHS is not an owner.
+  Finder->addMatcher(
+      binaryOperator(allOf(matchers::isAssignmentOperator(),
+                           hasLHS(unless(IsOwnerType)), hasRHS(CreatesOwner)))
+          .bind("bad_owner_creation_assignment"),
+      this);
+
+  // Matching on initialization operations where the initial value is a newly
+  // created owner, but the LHS is not an owner.
+  Finder->addMatcher(
+      namedDecl(varDecl(eachOf(allOf(hasInitializer(CreatesOwner),
+                                     unless(IsOwnerType)),
+                               allOf(hasInitializer(ConsideredOwner),
+                                     hasType(autoType().bind("deduced_type")))))
+                    .bind("bad_owner_creation_variable")),
+      this);
+
+  // Match on all function calls that expect owners as arguments, but didn't
+  // get them.
+  Finder->addMatcher(
+      callExpr(forEachArgumentWithParam(
+          expr(unless(ConsideredOwner)).bind("expected_owner_argument"),
+          parmVarDecl(IsOwnerType))),
+      this);
+
+  // Matching for function calls where one argument is a created owner, but the
+  // parameter type is not an owner.
+  Finder->addMatcher(callExpr(forEachArgumentWithParam(
+                         expr(CreatesOwner).bind("bad_owner_creation_argument"),
+                         parmVarDecl(unless(IsOwnerType))
+                             .bind("bad_owner_creation_parameter"))),
+                     this);
+
+  // Matching on functions, that return an owner/resource, but don't declare
+  // their return type as owner.
+  Finder->addMatcher(
+      functionDecl(
+          allOf(hasDescendant(returnStmt(hasReturnValue(ConsideredOwner))
+                                  .bind("bad_owner_return")),
+                unless(returns(qualType(hasDeclaration(OwnerDecl))))))
+          .bind("function_decl"),
+      this);
+
+  // Match on classes that have an owner as member, but don't declare a
+  // destructor to properly release the owner.
+  Finder->addMatcher(
+      cxxRecordDecl(
+          allOf(
+              has(fieldDecl(IsOwnerType).bind("undestructed_owner_member")),
+              anyOf(unless(has(cxxDestructorDecl())),
+                    has(cxxDestructorDecl(anyOf(isDefaulted(), isDeleted()))))))
+          .bind("non_destructor_class"),
+      this);
+}
+
+void OwningMemoryCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto &Nodes = Result.Nodes;
+
+  bool CheckExecuted = false;
+  CheckExecuted |= handleDeletion(Nodes);
+  CheckExecuted |= handleExpectedOwner(Nodes);
+  CheckExecuted |= handleAssignmentAndInit(Nodes);
+  CheckExecuted |= handleAssignmentFromNewOwner(Nodes);
+  CheckExecuted |= handleReturnValues(Nodes);
+  CheckExecuted |= handleOwnerMembers(Nodes);
+
+  assert(CheckExecuted &&
+         "None of the subroutines executed, logic error in matcher!");
+}
+
+bool OwningMemoryCheck::handleDeletion(const BoundNodes &Nodes) {
+  // Result of delete matchers.
+  const auto *DeleteStmt = Nodes.getNodeAs<CXXDeleteExpr>("delete_expr");
+  const auto *DeletedVariable =
+      Nodes.getNodeAs<DeclRefExpr>("deleted_variable");
+
+  // 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();
+    return true;
+  }
+  return false;
+}
+
+bool OwningMemoryCheck::handleExpectedOwner(const BoundNodes &Nodes) {
+  // Result of function call matchers.
+  const auto *ExpectedOwner = Nodes.getNodeAs<Expr>("expected_owner_argument");
+
+  // Expected function argument to be owner.
+  if (ExpectedOwner) {
+    diag(ExpectedOwner->getLocStart(),
+         "expected argument of type 'gsl::owner<>'; got %0")
+        << ExpectedOwner->getType() << ExpectedOwner->getSourceRange();
+    return true;
+  }
+  return false;
+}
+
+/// Assignment and initialization of owner variables.
+bool OwningMemoryCheck::handleAssignmentAndInit(const BoundNodes &Nodes) {
+  const auto *OwnerAssignment =
+      Nodes.getNodeAs<BinaryOperator>("owner_assignment");
+  const auto *OwnerInitialization =
+      Nodes.getNodeAs<VarDecl>("owner_initialization");
+  const auto *OwnerInitializer =
+      Nodes.getNodeAs<CXXCtorInitializer>("owner_member_initializer");
+
+  // Assignments to owners.
+  if (OwnerAssignment) {
+    diag(OwnerAssignment->getLocStart(),
+         "expected assignment source to be of type 'gsl::owner<>'; got %0")
+        << OwnerAssignment->getRHS()->getType()
+        << OwnerAssignment->getSourceRange();
+    return true;
+  }
+
+  // Initialization of owners.
+  if (OwnerInitialization) {
+    diag(OwnerInitialization->getLocStart(),
+         "expected initialization with value of type 'gsl::owner<>'; got %0")
+        << OwnerInitialization->getAnyInitializer()->getType()
+        << OwnerInitialization->getSourceRange();
+    return true;
+  }
+
+  // Initializer of class constructors that initialize owners.
+  if (OwnerInitializer) {
+    diag(OwnerInitializer->getSourceLocation(),
+         "expected initialization of owner member variable with value of type "
+         "'gsl::owner<>'; got %0")
+        // FIXME: the expression from getInit has type 'void', but the type
+        // of the supplied argument would be of interest.
+        << OwnerInitializer->getInit()->getType()
+        << OwnerInitializer->getSourceRange();
+    return true;
+  }
+  return false;
+}
+
+/// Problematic assignment and initializations, since the assigned value is a
+/// newly created owner.
+bool OwningMemoryCheck::handleAssignmentFromNewOwner(const BoundNodes &Nodes) {
+  const auto *BadOwnerAssignment =
+      Nodes.getNodeAs<BinaryOperator>("bad_owner_creation_assignment");
+  const auto *BadOwnerInitialization =
+      Nodes.getNodeAs<VarDecl>("bad_owner_creation_variable");
+
+  const auto *BadOwnerArgument =
+      Nodes.getNodeAs<Expr>("bad_owner_creation_argument");
+  const auto *BadOwnerParameter =
+      Nodes.getNodeAs<ParmVarDecl>("bad_owner_creation_parameter");
+
+  // Bad assignments to non-owners, where the RHS is a newly created owner.
+  if (BadOwnerAssignment) {
+    diag(BadOwnerAssignment->getLocStart(),
+         "assigning newly created 'gsl::owner<>' to non-owner %0")
+        << BadOwnerAssignment->getLHS()->getType()
+        << BadOwnerAssignment->getSourceRange();
+    return true;
+  }
+
+  // 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.
+
+    // 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;
+  }
+
+  // Function call, where one arguments is a newly created owner, but the
+  // parameter type is not.
+  if (BadOwnerArgument) {
+    assert(BadOwnerParameter &&
+           "parameter for the problematic argument not found");
+    diag(BadOwnerArgument->getLocStart(), "initializing non-owner argument of "
+                                          "type %0 with a newly created "
+                                          "'gsl::owner<>'")
+        << BadOwnerParameter->getType() << BadOwnerArgument->getSourceRange();
+    return true;
+  }
+  return false;
+}
+
+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.
+    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<>.
+    return true;
+  }
+  return false;
+}
+
+bool OwningMemoryCheck::handleOwnerMembers(const BoundNodes &Nodes) {
+  // Classes, that have owners as member, but do not declare destructors
+  // accordingly.
+  const auto *BadClass = Nodes.getNodeAs<CXXRecordDecl>("non_destructor_class");
+
+  // Classes, that contains owners, but do not declare destructors.
+  if (BadClass) {
+    const auto *DeclaredOwnerMember =
+        Nodes.getNodeAs<FieldDecl>("undestructed_owner_member");
+    assert(DeclaredOwnerMember &&
+           "match on class with bad destructor but without a declared owner");
+
+    diag(DeclaredOwnerMember->getLocStart(),
+         "member variable of type 'gsl::owner<>' requires the class %0 to "
+         "implement a destructor to release the owned resource")
+        << BadClass;
+    return true;
+  }
+  return false;
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h

@@ -0,0 +1,44 @@
+//===--- OwningMemoryCheck.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_CPPCOREGUIDELINES_OWNING_MEMORY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_OWNING_MEMORY_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Checks for common use cases for gsl::owner and enforces the unique owner
+/// nature of it whenever possible.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-owning-memory.html
+class OwningMemoryCheck : public ClangTidyCheck {
+public:
+  OwningMemoryCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  bool handleDeletion(const ast_matchers::BoundNodes &Nodes);
+  bool handleExpectedOwner(const ast_matchers::BoundNodes &Nodes);
+  bool handleAssignmentAndInit(const ast_matchers::BoundNodes &Nodes);
+  bool handleAssignmentFromNewOwner(const ast_matchers::BoundNodes &Nodes);
+  bool handleReturnValues(const ast_matchers::BoundNodes &Nodes);
+  bool handleOwnerMembers(const ast_matchers::BoundNodes &Nodes);
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_OWNING_MEMORY_H

clang-tools-extra/clang-tidy/utils/Matchers.h

@@ -17,6 +17,10 @@ namespace clang {
 namespace tidy {
 namespace matchers {
 
+AST_MATCHER(BinaryOperator, isAssignmentOperator) {
+  return Node.isAssignmentOp();
+}
+
 AST_MATCHER(BinaryOperator, isRelationalOperator) {
   return Node.isRelationalOp();
 }

clang-tools-extra/docs/ReleaseNotes.rst

@@ -114,6 +114,11 @@ Improvements to clang-tidy
   Finds cases where integer division in a floating point context is likely to
   cause unintended loss of precision.
 
+- New `cppcoreguidelines-owning-memory <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-owning-memory.html>`_ check 
+
+  This check implements the type-based semantic of ``gsl::owner<T*>``, but without
+  flow analysis.
+
 - New `hicpp-exception-baseclass
   <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-exception-baseclass.html>`_ check
 

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst

@@ -0,0 +1,143 @@
+.. title:: clang-tidy - cppcoreguidelines-owning-memory
+
+cppcoreguidelines-owning-memory
+===============================
+
+This check implements the type-based semantics of ``gsl::owner<T*>``, which allows 
+static analysis on code, that uses raw pointers to handle resources like 
+dynamic memory, but won't introduce RAII concepts.
+
+The relevant sections in the `C++ Core Guidelines <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md>`_ are I.11, C.33, R.3 and GSL.Views
+The definition of a ``gsl::owner<T*>`` is straight forward
+
+.. code-block:: c++
+
+  namespace gsl { template <typename T> owner = T; }
+
+It is therefore simple to introduce the owner even without using an implementation of
+the `Guideline Support Library <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#gsl-guideline-support-library>`_.
+
+All checks are purely type based and not (yet) flow sensitive.
+
+The following examples will demonstrate the correct and incorrect initializations
+of owners, assignment is handled the same way.
+
+.. code-block:: c++
+
+  // Creating an owner with factory functions is checked.
+  gsl::owner<int*> function_that_returns_owner() { return gsl::owner<int*>(new int(42)); }
+
+  // Dynamic memory must be assigned to an owner
+  int* Something = new int(42); // BAD, will be caught
+  gsl::owner<int*> Owner = new int(42); // Good
+  gsl::owner<int*> Owner = new int[42]; // Good as well
+
+  // Returned owner must be assigned to an owner
+  int* Something = function_that_returns_owner(); // Bad, factory function
+  gsl::owner<int*> Owner = function_that_returns_owner(); // Good, result lands in owner
+
+  // Something not a resource or owner should not be assigned to owners
+  int Stack = 42;
+  gsl::owner<int*> Owned = &Stack; // Bad, not a resource assigned
+
+In the case of dynamic memory as resource, only ``gsl::owner<T*>`` variables are allowed
+to be deleted.
+
+.. code-block:: c++
+
+  // Example Bad, non-owner as resource handle, will be caught.
+  int* NonOwner = new int(42); // First warning here, since new must land in an owner
+  delete NonOwner; // Second warning here, since only owners are allowed to be deleted
+
+  // Example Good, Ownership correclty stated
+  gsl::owner<int*> Owner = new int(42); // Good
+  delete Owner; // Good as well, statically enforced, that only owners get deleted
+  
+The check will furthermore ensure, that functions, that expect a ``gsl::owner<T*>`` as
+argument get called with either a ``gsl::owner<T*>`` or a newly created resource.
+
+.. code-block:: c++
+
+  void expects_owner(gsl::owner<int*> o) { delete o; }
+
+  // Bad Code
+  int NonOwner = 42;
+  expects_owner(&NonOwner); // Bad, will get caught
+
+  // Good Code
+  gsl::owner<int*> Owner = new int(42);
+  expects_owner(Owner); // Good
+  expects_owner(new int(42)); // Good as well, recognized created resource
+
+Limitations
+-----------
+
+Using ``gsl::owner<T*>`` in a typedef or alias is not handled correctly. 
+
+.. code-block:: c++
+
+  using heap_int = gsl::owner<int*>;
+  heap_int allocated = new int(42); // False positive!
+
+The ``gsl::owner<T*>`` is declared as a templated type alias.
+In template functions and classes, like in the example below, the information
+of the type aliases gets lost. Therefore using ``gsl::owner<T*>`` in a heavy templated
+code base might lead to false positives.
+
+.. code-block:: c++
+
+  // This template function works as expected. Type information doesn't get lost.
+  template <typename T>
+  void delete_owner(gsl::owner<T*> owned_object) {
+    delete owned_object; // Everything alright
+  }
+
+  gsl::owner<int*> function_that_returns_owner() { return gsl::owner<int*>(new int(42)); }
+
+  // Type deduction does not work for auto variables. 
+  // This is caught by the check and will be noted accordingly.
+  auto OwnedObject = function_that_returns_owner(); // Type of OwnedObject will be int*
+
+  // Problematic function template that looses the typeinformation on owner
+  template <typename T>
+  void bad_template_function(T some_object) {
+    // This line will trigger the warning, that a non-owner is assigned to an owner
+    gsl::owner<T*> new_owner = some_object;
+  }
+
+  // Calling the function with an owner still yields a false positive.
+  bad_template_function(gsl::owner<int*>(new int(42)));
+
+
+  // The same issue occurs with templated classes like the following.
+  template <typename T>
+  class OwnedValue {
+  public:
+    const T getValue() const { return _val; }
+  private:
+    T _val;
+  };
+
+  // Code, that yields a false positive.
+  OwnedValue<gsl::owner<int*>> Owner(new int(42)); // Type deduction yield T -> int * 
+  // False positive, getValue returns int* and not gsl::owner<int*>
+  gsl::owner<int*> OwnedInt = Owner.getValue(); 
+
+Another limitation of the current implementation is only the type based checking.
+Suppose you have code like the following:
+
+.. code-block:: c++
+
+  // Two owners with assigned resources
+  gsl::owner<int*> Owner1 = new int(42); 
+  gsl::owner<int*> Owner2 = new int(42);
+
+  Owner2 = Owner1; // Conceptual Leak of initial resource of Owner2!
+  Owner1 = nullptr;
+
+The semantic of a ``gsl::owner<T*>`` is mostly like a ``std::unique_ptr<T>``, therefore
+assignment of two ``gsl::owner<T*>`` is considered a move, which requires that the 
+resource ``Owner2`` must have been released before the assignment.
+This kind of condition could be catched in later improvements of this check with 
+flowsensitive analysis. Currently, the `Clang Static Analyzer` catches this bug
+for dynamic memory, but not for general types of resources.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -41,6 +41,7 @@ Clang-Tidy Checks
    cppcoreguidelines-c-copy-assignment-signature
    cppcoreguidelines-interfaces-global-init
    cppcoreguidelines-no-malloc
+   cppcoreguidelines-owning-memory
    cppcoreguidelines-pro-bounds-array-to-pointer-decay
    cppcoreguidelines-pro-bounds-constant-array-index
    cppcoreguidelines-pro-bounds-pointer-arithmetic

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

@@ -0,0 +1,388 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-owning-memory %t
+
+namespace gsl {
+template <class T>
+using owner = T;
+} // namespace gsl
+
+template <typename T>
+class unique_ptr {
+public:
+  unique_ptr(gsl::owner<T> resource) : memory(resource) {}
+  unique_ptr(const unique_ptr<T> &) = default;
+
+  ~unique_ptr() { delete memory; }
+
+private:
+  gsl::owner<T> memory;
+};
+
+void takes_owner(gsl::owner<int *> owned_int) {
+}
+
+void takes_pointer(int *unowned_int) {
+}
+
+void takes_owner_and_more(int some_int, gsl::owner<int *> owned_int, float f) {
+}
+
+template <typename T>
+void takes_templated_owner(gsl::owner<T> owned_T) {
+}
+
+gsl::owner<int *> returns_owner1() { return gsl::owner<int *>(new int(42)); } // Ok
+gsl::owner<int *> returns_owner2() { return new int(42); }                    // Ok
+
+int *returns_no_owner1() { return nullptr; }
+int *returns_no_owner2() {
+  return new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a newly created resource of type 'int *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
+}
+int *returns_no_owner3() {
+  int *should_be_owner = new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+  return should_be_owner;
+}
+int *returns_no_owner4() {
+  gsl::owner<int *> owner = new int(42);
+  return owner;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: returning a newly created resource of type 'int *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
+}
+
+unique_ptr<int *> returns_no_owner5() {
+  return unique_ptr<int *>(new int(42)); // Ok
+}
+
+/// FIXME: CSA finds it, but the report is misleading. Ownersemantics can catch this
+/// by flow analysis similar to misc-use-after-move.
+void csa_not_finding_leak() {
+  gsl::owner<int *> o1 = new int(42); // Ok
+
+  gsl::owner<int *> o2 = o1; // Ok
+  o2 = new int(45);          // conceptual leak, the memory from o1 is now leaked, since its considered moved in the guidelines
+
+  delete o2;
+  // actual leak occurs here, its found, but mixed
+  delete o1;
+}
+
+void test_assignment_and_initialization() {
+  int stack_int1 = 15;
+  int stack_int2;
+
+  gsl::owner<int *> owned_int1 = &stack_int1; // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner<int *> owned_int2;
+  owned_int2 = &stack_int2; // BAD since no owner, bad since uninitialized
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner<int *> owned_int3 = new int(42); // Good
+  owned_int3 = nullptr;                       // Good
+
+  gsl::owner<int *> owned_int4(nullptr); // Ok
+  owned_int4 = new int(42);              // Good
+
+  gsl::owner<int *> owned_int5 = owned_int3; // Good
+
+  gsl::owner<int *> owned_int6{nullptr}; // Ok
+  owned_int6 = owned_int4;               // Good
+
+  // FIXME:, flow analysis for the case of reassignment. Value must be released before
+  owned_int6 = owned_int3; // BAD, because reassignment without resource release
+
+  auto owned_int7 = returns_owner1(); // Bad, since type deduction eliminates the owner wrapper
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  const auto owned_int8 = returns_owner2(); // Bad, since type deduction eliminates the owner wrapper
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *const' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  gsl::owner<int *> owned_int9 = returns_owner1(); // Ok
+  int *unowned_int3 = returns_owner1();            // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+
+  gsl::owner<int *> owned_int10;
+  owned_int10 = returns_owner1(); // Ok
+
+  int *unowned_int4;
+  unowned_int4 = returns_owner1(); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: assigning newly created 'gsl::owner<>' to non-owner 'int *'
+
+  gsl::owner<int *> owned_int11 = returns_no_owner1(); // Bad since no owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner<int *> owned_int12;
+  owned_int12 = returns_no_owner1(); // Bad since no owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
+
+  int *unowned_int5 = returns_no_owner1(); // Ok
+  int *unowned_int6;
+  unowned_int6 = returns_no_owner1(); // Ok
+
+  int *unowned_int7 = new int(42); // Bad, since resource not assigned to an owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
+
+  int *unowned_int8;
+  unowned_int8 = new int(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: assigning newly created 'gsl::owner<>' to non-owner 'int *'
+
+  gsl::owner<int *> owned_int13 = nullptr; // Ok
+}
+
+void test_deletion() {
+  gsl::owner<int *> owned_int1 = new int(42);
+  delete owned_int1; // Good
+
+  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
+
+  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
+
+  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;
+  takes_owner(&stack_int); // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: expected argument of type 'gsl::owner<>'; got 'int *'
+  takes_owner(unowned_int1); // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: expected argument of type 'gsl::owner<>'; got 'int *'
+
+  gsl::owner<int *> owned_int1 = new int(42);
+  takes_owner(owned_int1); // Ok
+
+  takes_owner_and_more(42, &stack_int, 42.0f); // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: expected argument of type 'gsl::owner<>'; got 'int *'
+  takes_owner_and_more(42, unowned_int1, 42.0f); // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: expected argument of type 'gsl::owner<>'; got 'int *'
+
+  takes_owner_and_more(42, new int(42), 42.0f); // Ok, since new is consumed by owner
+  takes_owner_and_more(42, owned_int1, 42.0f);  // Ok, since owner as argument
+
+  takes_templated_owner(owned_int1);   // Ok
+  takes_templated_owner(new int(42));  // Ok
+  takes_templated_owner(unowned_int1); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: expected argument of type 'gsl::owner<>'; got 'int *'
+
+  takes_owner(returns_owner1());    // Ok
+  takes_owner(returns_no_owner1()); // BAD
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: expected argument of type 'gsl::owner<>'; got 'int *'
+}
+
+void test_unowned_function_calls() {
+  int stack_int = 42;
+  int *unowned_int1 = &stack_int;
+  gsl::owner<int *> owned_int1 = new int(42);
+
+  takes_pointer(&stack_int);   // Ok
+  takes_pointer(unowned_int1); // Ok
+  takes_pointer(owned_int1);   // Ok
+  takes_pointer(new int(42));  // Bad, since new creates and owner
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: initializing non-owner argument of type 'int *' with a newly created 'gsl::owner<>'
+
+  takes_pointer(returns_owner1()); // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: initializing non-owner argument of type 'int *' with a newly created 'gsl::owner<>'
+
+  takes_pointer(returns_no_owner1()); // Ok
+}
+
+// FIXME: Typedefing owner<> to something else does not work.
+// This might be necessary for code already having a similar typedef like owner<> and
+// replacing it with owner<>. This might be the same problem as with templates.
+// The canonical type will ignore the owner<> alias, since its a typedef as well.
+//
+// Check, if owners hidden by typedef are handled the same as 'obvious' owners.
+#if 0
+using heap_int = gsl::owner<int *>;
+typedef gsl::owner<float *> heap_float;
+
+// This tests only a subset, assuming that the check will either see through the
+// typedef or not (it doesn't!).
+void test_typedefed_values() {
+  // Modern typedef.
+  int StackInt1 = 42;
+  heap_int HeapInt1 = &StackInt1;
+  // CHECK MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'int *'
+
+  //FIXME: Typedef not considered correctly here.
+  // heap_int HeapInt2 = new int(42); // Ok
+  takes_pointer(HeapInt1); // Ok
+  takes_owner(HeapInt1);   // Ok
+
+  // Traditional typedef.
+  float StackFloat1 = 42.0f;
+  heap_float HeapFloat1 = &StackFloat1;
+  // CHECK MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'float *'
+
+  //FIXME: Typedef not considered correctly here.
+  // heap_float HeapFloat2 = new float(42.0f);
+  HeapFloat2 = HeapFloat1; // Ok
+}
+#endif
+
+struct ArbitraryClass {};
+struct ClassWithOwner {                    // Does not define destructor, necessary with owner
+  ClassWithOwner() : owner_var(nullptr) {} // Ok
+
+  ClassWithOwner(ArbitraryClass &other) : owner_var(&other) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:43: warning: expected initialization of owner member variable with value of type 'gsl::owner<>'; got 'ArbitraryClass *'
+
+  ClassWithOwner(gsl::owner<ArbitraryClass *> other) : owner_var(other) {} // Ok
+
+  ClassWithOwner(gsl::owner<ArbitraryClass *> data, int /* unused */) { // Ok
+    owner_var = data;                                                   // Ok
+  }
+
+  ClassWithOwner(ArbitraryClass *bad_data, int /* unused */, int /* unused */) {
+    owner_var = bad_data;
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: expected assignment source to be of type 'gsl::owner<>'; got 'ArbitraryClass *'
+  }
+
+  ClassWithOwner(ClassWithOwner &&other) : owner_var{other.owner_var} {} // Ok
+
+  ClassWithOwner &operator=(ClassWithOwner &&other) {
+    owner_var = other.owner_var; // Ok
+    return *this;
+  }
+
+  // Returning means, that the owner is "moved", so the class should not access this
+  // variable anymore after this method gets called.
+  gsl::owner<ArbitraryClass *> buggy_but_returns_owner() { return owner_var; }
+
+  gsl::owner<ArbitraryClass *> owner_var;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: member variable of type 'gsl::owner<>' requires the class 'ClassWithOwner' to implement a destructor to release the owned resource
+};
+
+class DefaultedDestructor {         // Bad since default constructor with owner
+  ~DefaultedDestructor() = default; // Bad, since will not destroy the owner
+  gsl::owner<int *> Owner;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: member variable of type 'gsl::owner<>' requires the class 'DefaultedDestructor' to implement a destructor to release the owned resource
+};
+
+struct DeletedDestructor {
+  ~DeletedDestructor() = delete;
+  gsl::owner<int *> Owner;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: member variable of type 'gsl::owner<>' requires the class 'DeletedDestructor' to implement a destructor to release the owned resource
+};
+
+void test_class_with_owner() {
+  ArbitraryClass A;
+  ClassWithOwner C1;                                                   // Ok
+  ClassWithOwner C2{A};                                                // Bad, since the owner would be initialized with an non-owner, but catched in the class
+  ClassWithOwner C3{gsl::owner<ArbitraryClass *>(new ArbitraryClass)}; // Ok
+
+  const auto Owner1 = C3.buggy_but_returns_owner(); // BAD, deduces Owner1 to ArbitraryClass *const
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'ArbitraryClass *const' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  auto Owner2 = C2.buggy_but_returns_owner(); // BAD, deduces Owner2 to ArbitraryClass *
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: initializing non-owner 'ArbitraryClass *' with a newly created 'gsl::owner<>'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: note: type deduction did not result in an owner
+
+  Owner2 = &A; // Ok, since type deduction did NOT result in owner<int*>
+
+  gsl::owner<ArbitraryClass *> Owner3 = C1.buggy_but_returns_owner(); // Ok, still an owner
+  Owner3 = &A;                                                        // Bad, since assignment of non-owner to owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'ArbitraryClass *'
+}
+
+template <typename T>
+struct HeapArray {                                          // Ok, since destructor with owner
+  HeapArray() : _data(nullptr), size(0) {}                  // Ok
+  HeapArray(int size) : _data(new int[size]), size(size) {} // Ok
+  HeapArray(int size, T val) {
+    _data = new int[size]; // Ok
+    size = size;
+    for (auto i = 0u; i < size; ++i)
+      _data[i] = val; // Ok
+  }
+  HeapArray(int size, T val, int *problematic) : _data{problematic}, size(size) {} // Bad
+  // CHECK-MESSAGES: [[@LINE-1]]:50: warning: expected initialization of owner member variable with value of type 'gsl::owner<>'; got 'void'
+  // FIXME: void is incorrect type, probably wrong thing matched
+
+  HeapArray(HeapArray &&other) : _data(other._data), size(other.size) { // Ok
+    other._data = nullptr;                                              // Ok
+    other.size = 0;
+  }
+
+  HeapArray<T> &operator=(HeapArray<T> &&other) {
+    _data = other._data; // Ok, NOLINT warning here about bad types, why?
+    size = other.size;
+    return *this;
+  }
+
+  ~HeapArray() { delete[] _data; } // Ok
+
+  T *data() { return _data; } // Ok NOLINT, because it "looks" like a factory
+
+  gsl::owner<T *> _data;
+  unsigned int size;
+};
+
+void test_inner_template() {
+  HeapArray<int> Array1;
+  HeapArray<int> Array2(100);
+  HeapArray<int> Array3(100, 0);
+
+  Array1 = static_cast<HeapArray<int> &&>(Array2);
+  HeapArray<int> Array4(static_cast<HeapArray<int> &&>(Array3));
+
+  int *NonOwningPtr = Array1.data();           // Ok
+  gsl::owner<int *> OwningPtr = Array1.data(); // Bad, since it does not return the owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+}
+
+// FIXME: Typededuction removes the owner - wrapper, therefore gsl::owner can not be used
+// with Template classes like this. Is there a walkaround?
+template <typename T>
+struct TemplateValue {
+  TemplateValue() = default;
+  TemplateValue(T t) : val{t} {}
+
+  void setVal(const T &t) { val = t; }
+  const T getVal() const { return val; }
+
+  T val;
+};
+
+// FIXME: Same typededcution problems
+template <typename T>
+void template_function(T t) {
+  gsl::owner<int *> owner_t = t; // Probably bad, since type deduction still wrong
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'T'
+  // CHECK-MESSAGES: [[@LINE-2]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+}
+
+// FIXME: Same typededcution problems
+void test_templates() {
+  int stack_int = 42;
+  int *stack_ptr1 = &stack_int;
+
+  TemplateValue<gsl::owner<int *>> Owner0; // Ok, T should be owner, but is int*
+
+  TemplateValue<gsl::owner<int *>> Owner1(new int(42)); // Ok, T should be owner, but is int*
+  Owner1.setVal(&stack_int);                            // Bad since non-owner assignment
+  Owner1.setVal(stack_ptr1);                            // Bad since non-owner assignment
+  //Owner1.setVal(new int(42)); // Ok, but since type deduction is wrong, this one is considered harmful
+
+  int *stack_ptr2 = Owner1.getVal(); // Bad, initializing non-owner with owner
+
+  TemplateValue<int *> NonOwner1(new int(42));      // Bad, T is int *, hence dynamic memory to non-owner
+  gsl::owner<int *> IntOwner1 = NonOwner1.getVal(); // Bad, since owner initialized with non-owner
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expected initialization with value of type 'gsl::owner<>'; got 'int *'
+
+  template_function(IntOwner1);  // Ok, but not actually ok, since type deduction removes owner
+  template_function(stack_ptr1); // Bad, but type deduction gets it wrong
+}