[clang-tools-extra] r313059 - [clang-tidy] Revert Implement type-based check for gsl::owner
Jonas Toth via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 12 11:35:54 PDT 2017
Author: jonastoth
Date: Tue Sep 12 11:35:54 2017
New Revision: 313059
URL: http://llvm.org/viewvc/llvm-project?rev=313059&view=rev
Log:
[clang-tidy] Revert Implement type-based check for gsl::owner
This should unbreak the buildbot for visual studio 2015 for now.
Removed:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-owning-memory.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/utils/Matchers.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=313059&r1=313058&r2=313059&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt Tue Sep 12 11:35:54 2017
@@ -4,7 +4,6 @@ add_clang_library(clangTidyCppCoreGuidel
CppCoreGuidelinesTidyModule.cpp
InterfacesGlobalInitCheck.cpp
NoMallocCheck.cpp
- OwningMemoryCheck.cpp
ProBoundsArrayToPointerDecayCheck.cpp
ProBoundsConstantArrayIndexCheck.cpp
ProBoundsPointerArithmeticCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=313059&r1=313058&r2=313059&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp Tue Sep 12 11:35:54 2017
@@ -13,7 +13,6 @@
#include "../misc/UnconventionalAssignOperatorCheck.h"
#include "InterfacesGlobalInitCheck.h"
#include "NoMallocCheck.h"
-#include "OwningMemoryCheck.h"
#include "ProBoundsArrayToPointerDecayCheck.h"
#include "ProBoundsConstantArrayIndexCheck.h"
#include "ProBoundsPointerArithmeticCheck.h"
@@ -38,8 +37,6 @@ 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>(
Removed: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp?rev=313058&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp (removed)
@@ -1,315 +0,0 @@
-//===--- 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
Removed: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h?rev=313058&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h (removed)
@@ -1,44 +0,0 @@
-//===--- 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
Modified: clang-tools-extra/trunk/clang-tidy/utils/Matchers.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/Matchers.h?rev=313059&r1=313058&r2=313059&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/Matchers.h (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/Matchers.h Tue Sep 12 11:35:54 2017
@@ -17,10 +17,6 @@ namespace clang {
namespace tidy {
namespace matchers {
-AST_MATCHER(BinaryOperator, isAssignmentOperator) {
- return Node.isAssignmentOp();
-}
-
AST_MATCHER(BinaryOperator, isRelationalOperator) {
return Node.isRelationalOp();
}
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=313059&r1=313058&r2=313059&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue Sep 12 11:35:54 2017
@@ -114,11 +114,6 @@ 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
Removed: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst?rev=313058&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-owning-memory.rst (removed)
@@ -1,143 +0,0 @@
-.. 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.
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=313059&r1=313058&r2=313059&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Sep 12 11:35:54 2017
@@ -41,7 +41,6 @@ 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
Removed: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-owning-memory.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-owning-memory.cpp?rev=313058&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-owning-memory.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-owning-memory.cpp (removed)
@@ -1,388 +0,0 @@
-// 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
-}
More information about the cfe-commits
mailing list