PiotrZSL updated this revision to Diff 510132. PiotrZSL added a comment. Fix doc
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147357/new/ https://reviews.llvm.org/D147357 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp @@ -0,0 +1,52 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t + +namespace std { + template<typename T> + struct optional + { + constexpr optional() noexcept; + constexpr optional(T&&) noexcept; + constexpr optional(const T&) noexcept; + template<typename U> + constexpr optional(U&&) noexcept; + const T &operator*() const; + const T &value() const; + T value_or(T) const; + }; +} + +void takeOptionalValue(std::optional<int>); +void takeOptionalRef(const std::optional<int>&); +void takeOtherOptional(std::optional<long>); + +void incorrect(std::optional<int> param) +{ + takeOptionalValue(*param); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion] + + takeOptionalValue(param.value()); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion] + + takeOptionalRef(*param); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion] + + takeOptionalRef(param.value()); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion] + std::optional<int> p = *param; + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion] +} + +void correct(std::optional<int> param) +{ + takeOtherOptional(*param); + + takeOtherOptional(param.value()); + + takeOtherOptional(param.value_or(5U)); + + std::optional<long> p = *param; + + takeOptionalValue(param.value_or(5U)); + + takeOptionalRef(param.value_or(5U)); +} Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -105,6 +105,7 @@ `bugprone-multiple-statement-macro <bugprone/multiple-statement-macro.html>`_, `bugprone-no-escape <bugprone/no-escape.html>`_, `bugprone-not-null-terminated-result <bugprone/not-null-terminated-result.html>`_, "Yes" + `bugprone-optional-value-conversion <bugprone/optional-value-conversion.html>`_, `bugprone-parent-virtual-call <bugprone/parent-virtual-call.html>`_, "Yes" `bugprone-posix-return <bugprone/posix-return.html>`_, "Yes" `bugprone-redundant-branch-condition <bugprone/redundant-branch-condition.html>`_, "Yes" Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst @@ -0,0 +1,49 @@ +.. title:: clang-tidy - bugprone-optional-value-conversion + +bugprone-optional-value-conversion +================================== + +Detects potentially unintentional and redundant conversions where a value is +extracted from an optional-like type and then used to create a new instance of +the same optional-like type. + +These conversions might be the result of developer oversight, leftovers from +code refactoring, or other situations that could lead to unintended exceptions +or cases where the resulting optional is always initialized, which might be +unexpected behavior. + +.. code-block:: c++ + + #include <optional> + + void print(std::optional<int>); + + int main() + { + std::optional<int> opt; + // ... + + // Unintentional conversion from std::optional<int> to int and back to + // std::optional<int>: + print(opt.value()); + + // ... + } + +Value extraction using ``operator *`` is matched by default. +Check does not provide auto-fixes. + +Options: +-------- + +.. option:: OptionalTypes + + Semicolon-separated list of (fully qualified) optional type names or regular + expressions that match the optional types. + Default value is `::std::optional;::absl::optional;::boost::optional`. + +.. option:: ValueMethods + + Semicolon-separated list of (fully qualified) method names or regular + expressions that match the methods. + Default value is `::value$;::get$`. Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -101,6 +101,13 @@ New checks ^^^^^^^^^^ +- New :doc:`bugprone-optional-value-conversion + <clang-tidy/checks/bugprone/optional-value-conversion>` check. + + Detects potentially unintentional and redundant conversions where a value is + extracted from an optional-like type and then used to create a new instance + of the same optional-like type. + - New :doc:`bugprone-unsafe-functions <clang-tidy/checks/bugprone/unsafe-functions>` check. Index: clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h @@ -0,0 +1,38 @@ +//===--- OptionalValueConversionCheck.h - clang-tidy ------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H + +#include "../ClangTidyCheck.h" +#include <vector> + +namespace clang::tidy::bugprone { + +/// Detects potentially unintentional and redundant conversions where a value is +/// extracted from an optional-like type and then used to create a new instance +/// of the same optional-like type. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/optional-value-conversion.html +class OptionalValueConversionCheck : public ClangTidyCheck { +public: + OptionalValueConversionCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + std::optional<TraversalKind> getCheckTraversalKind() const override; + +private: + std::vector<StringRef> OptionalTypes; + std::vector<StringRef> ValueMethods; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H Index: clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp @@ -0,0 +1,93 @@ +//===--- OptionalValueConversionCheck.cpp - clang-tidy --------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "OptionalValueConversionCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { + +AST_MATCHER_P(QualType, hasCleanType, ast_matchers::internal::Matcher<QualType>, + InnerMatcher) { + return InnerMatcher.matches( + Node.getNonReferenceType().getUnqualifiedType().getCanonicalType(), + Finder, Builder); +} + +} // namespace + +OptionalValueConversionCheck::OptionalValueConversionCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + OptionalTypes(utils::options::parseStringList( + Options.get("OptionalTypes", + "::std::optional;::absl::optional;::boost::optional"))), + ValueMethods(utils::options::parseStringList( + Options.get("ValueMethods", "::value$;::get$"))) {} + +std::optional<TraversalKind> +OptionalValueConversionCheck::getCheckTraversalKind() const { + return TK_AsIs; +} + +void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) { + auto ConstructTypeMatcher = + qualType(hasCleanType(qualType().bind("optional-type"))); + + auto CallTypeMatcher = + qualType(hasCleanType(equalsBoundNode("optional-type"))); + + auto OptionalDereferenceMatcher = callExpr( + anyOf( + cxxOperatorCallExpr(hasOverloadedOperatorName("*"), + hasUnaryOperand(hasType(CallTypeMatcher))), + cxxMemberCallExpr(thisPointerType(CallTypeMatcher), + callee(cxxMethodDecl( + matchers::matchesAnyListedName(ValueMethods)))) + + ), + hasType(qualType().bind("value-type"))); + + Finder->addMatcher( + cxxConstructExpr( + argumentCountIs(1U), + hasDeclaration(cxxConstructorDecl( + ofClass(matchers::matchesAnyListedName(OptionalTypes)))), + hasType(ConstructTypeMatcher), + hasArgument(0U, ignoringImpCasts(OptionalDereferenceMatcher))) + .bind("expr"), + this); +} + +void OptionalValueConversionCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "OptionalTypes", + utils::options::serializeStringList(OptionalTypes)); + Options.store(Opts, "ValueMethods", + utils::options::serializeStringList(ValueMethods)); +} + +void OptionalValueConversionCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedExpr = Result.Nodes.getNodeAs<Expr>("expr"); + const auto *OptionalType = Result.Nodes.getNodeAs<QualType>("optional-type"); + const auto *ValueType = Result.Nodes.getNodeAs<QualType>("value-type"); + + diag(MatchedExpr->getExprLoc(), + "conversion from %0 into %1 and back into %0, remove potentially " + "error-prone optional dereference") + << *OptionalType << ValueType->getUnqualifiedType(); +} + +} // namespace clang::tidy::bugprone Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -34,6 +34,7 @@ MultipleStatementMacroCheck.cpp NoEscapeCheck.cpp NotNullTerminatedResultCheck.cpp + OptionalValueConversionCheck.cpp ParentVirtualCallCheck.cpp PosixReturnCheck.cpp RedundantBranchConditionCheck.cpp Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -39,6 +39,7 @@ #include "MultipleStatementMacroCheck.h" #include "NoEscapeCheck.h" #include "NotNullTerminatedResultCheck.h" +#include "OptionalValueConversionCheck.h" #include "ParentVirtualCallCheck.h" #include "PosixReturnCheck.h" #include "RedundantBranchConditionCheck.h" @@ -134,6 +135,8 @@ "bugprone-move-forwarding-reference"); CheckFactories.registerCheck<MultipleStatementMacroCheck>( "bugprone-multiple-statement-macro"); + CheckFactories.registerCheck<OptionalValueConversionCheck>( + "bugprone-optional-value-conversion"); CheckFactories.registerCheck<RedundantBranchConditionCheck>( "bugprone-redundant-branch-condition"); CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits