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 -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits