https://github.com/gamesh411 updated https://github.com/llvm/llvm-project/pull/200166
From 31b5a84b204d3297040e2677c9d8ea9fbf99a42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <[email protected]> Date: Thu, 28 May 2026 12:00:10 +0200 Subject: [PATCH 1/3] [clang-tidy] Add performance-expensive-value-or check Warn when value_or() is called on an optional type whose underlying value type is expensive to copy (not trivially copyable, or larger than a configurable size threshold). While value() and operator* return references, value_or() always returns by value. --- .../clang-tidy/performance/CMakeLists.txt | 1 + .../performance/ExpensiveValueOrCheck.cpp | 70 +++++++++++ .../performance/ExpensiveValueOrCheck.h | 40 +++++++ .../performance/PerformanceTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/performance/expensive-value-or.rst | 62 ++++++++++ .../performance/expensive-value-or-absl.cpp | 16 +++ .../performance/expensive-value-or-rvalue.cpp | 24 ++++ .../performance/expensive-value-or.cpp | 110 ++++++++++++++++++ 10 files changed, 333 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/performance/expensive-value-or.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-absl.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-rvalue.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or.cpp diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt index 0c778b5a9f36b..913421996f505 100644 --- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt @@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidyPerformanceModule STATIC AvoidEndlCheck.cpp EnumSizeCheck.cpp + ExpensiveValueOrCheck.cpp ForRangeCopyCheck.cpp ImplicitConversionInLoopCheck.cpp InefficientAlgorithmCheck.cpp diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp new file mode 100644 index 0000000000000..aa9e412e99bce --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp @@ -0,0 +1,70 @@ +//===----------------------------------------------------------------------===// +// +// 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 "ExpensiveValueOrCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::performance { + +ExpensiveValueOrCheck::ExpensiveValueOrCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + SizeThreshold(Options.get("SizeThreshold", 8U)), + WarnOnRvalueOptional(Options.get("WarnOnRvalueOptional", false)), + OptionalTypes(utils::options::parseStringList( + Options.get("OptionalTypes", "::std::optional"))) {} + +void ExpensiveValueOrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "SizeThreshold", SizeThreshold); + Options.store(Opts, "WarnOnRvalueOptional", WarnOnRvalueOptional); + Options.store(Opts, "OptionalTypes", + utils::options::serializeStringList(OptionalTypes)); +} + +void ExpensiveValueOrCheck::registerMatchers(MatchFinder *Finder) { + auto OptionalTypesMatcher = hasAnyName(OptionalTypes); + + Finder->addMatcher( + cxxMemberCallExpr(callee(cxxMethodDecl(hasName("value_or"), + ofClass(OptionalTypesMatcher)))) + .bind("call"), + this); +} + +void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call"); + if (!Call) + return; + + const Expr *ObjExpr = Call->getImplicitObjectArgument(); + if (!WarnOnRvalueOptional && ObjExpr && !ObjExpr->isLValue()) + return; + + QualType ValueType = Call->getType().getCanonicalType(); + if (ValueType->isDependentType() || ValueType->isIncompleteType()) + return; + + const ASTContext &Ctx = *Result.Context; + int64_t ValueSize = Ctx.getTypeSizeInChars(ValueType).getQuantity(); + bool IsExpensive = !ValueType.isTriviallyCopyableType(Ctx) || + ValueSize > static_cast<int64_t>(SizeThreshold); + + if (!IsExpensive) + return; + + diag(Call->getExprLoc(), + "'value_or' copies expensive type %0; consider using 'operator*' or " + "'value()' with a separate fallback") + << ValueType; +} + +} // namespace clang::tidy::performance diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h new file mode 100644 index 0000000000000..8b46b6d4a9ff0 --- /dev/null +++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h @@ -0,0 +1,40 @@ +//===----------------------------------------------------------------------===// +// +// 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_PERFORMANCE_EXPENSIVEVALUEORCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_EXPENSIVEVALUEORCHECK_H + +#include "../ClangTidyCheck.h" +#include <vector> + +namespace clang::tidy::performance { + +/// Warns when 'value_or' is called on an optional type whose underlying type +/// is expensive to copy (not trivially copyable, or larger than a threshold). +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/performance/expensive-value-or.html +class ExpensiveValueOrCheck : public ClangTidyCheck { +public: + ExpensiveValueOrCheck(StringRef Name, ClangTidyContext *Context); + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const unsigned SizeThreshold; + const bool WarnOnRvalueOptional; + const std::vector<StringRef> OptionalTypes; +}; + +} // namespace clang::tidy::performance + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_EXPENSIVEVALUEORCHECK_H diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp index a4c1cdacab496..9eee02494be91 100644 --- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidyModule.h" #include "AvoidEndlCheck.h" #include "EnumSizeCheck.h" +#include "ExpensiveValueOrCheck.h" #include "ForRangeCopyCheck.h" #include "ImplicitConversionInLoopCheck.h" #include "InefficientAlgorithmCheck.h" @@ -39,6 +40,8 @@ class PerformanceModule : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<AvoidEndlCheck>("performance-avoid-endl"); CheckFactories.registerCheck<EnumSizeCheck>("performance-enum-size"); + CheckFactories.registerCheck<ExpensiveValueOrCheck>( + "performance-expensive-value-or"); CheckFactories.registerCheck<PreferSingleCharOverloadsCheck>( "performance-faster-string-find"); CheckFactories.registerCheck<ForRangeCopyCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0b3bb091307e7..da0ac85f03eb0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -275,6 +275,12 @@ New checks Finds places where structured bindings could be used to decompose pairs and suggests replacing them. +- New :doc:`performance-expensive-value-or + <clang-tidy/checks/performance/expensive-value-or>` check. + + Warns when ``value_or`` is called on an optional type whose underlying value + type is expensive to copy. + - New :doc:`performance-string-view-conversions <clang-tidy/checks/performance/string-view-conversions>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 937b80da9b601..974655cff268c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -352,6 +352,7 @@ Clang-Tidy Checks :doc:`openmp-use-default-none <openmp/use-default-none>`, :doc:`performance-avoid-endl <performance/avoid-endl>`, "Yes" :doc:`performance-enum-size <performance/enum-size>`, + :doc:`performance-expensive-value-or <performance/expensive-value-or>`, :doc:`performance-for-range-copy <performance/for-range-copy>`, "Yes" :doc:`performance-implicit-conversion-in-loop <performance/implicit-conversion-in-loop>`, :doc:`performance-inefficient-algorithm <performance/inefficient-algorithm>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/expensive-value-or.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/expensive-value-or.rst new file mode 100644 index 0000000000000..0592758a53381 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/expensive-value-or.rst @@ -0,0 +1,62 @@ +.. title:: clang-tidy - performance-expensive-value-or + +performance-expensive-value-or +============================== + +Finds calls to ``value_or`` on optional types where the underlying value type +is expensive to copy. While ``value()`` and ``operator*`` return references, +``value_or`` always returns by value, which involves copying the contained +value. + +The check is applied to types that are not trivially copyable or whose size +exceeds a configurable threshold. It supports ``std::optional``, +``boost::optional``, ``absl::optional``, and other optional-like types via +configuration. + +Example: + +.. code-block:: c++ + + #include <optional> + #include <string> + + void example(std::optional<std::string> opt) { + // Warning: copies the std::string out of the optional. + auto val = opt.value_or("default"); + + // Alternatives that avoid the copy: + const std::string fallback = "default"; + const auto &ref = opt.has_value() ? *opt : fallback; + } + +Options +------- + +.. option:: SizeThreshold + + The minimum size in bytes (exclusive) above which a trivially-copyable type + is considered expensive to copy. Types with ``sizeof(T) > SizeThreshold`` + trigger the warning even if they are trivially copyable. Types at or below + this threshold only trigger if they are not trivially copyable. + Default is `8`. + +.. option:: OptionalTypes + + Semicolon-separated list of fully-qualified names of optional-like class + templates to check. The check matches calls to ``value_or`` on + specializations of these templates. Default is ``::std::optional``. + + Example configuration to also check ``absl::optional``: + + .. code-block:: yaml + + CheckOptions: + performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional" + +.. option:: WarnOnRvalueOptional + + When `false` (default), the check does not warn when ``value_or`` is called + on an rvalue optional (e.g., a function return value). The rationale is that + rvalue overloads of ``value_or`` may move instead of copy, and replacing the + call would require materializing the temporary into a named variable to check + it before dereferencing. Set to `true` to warn in all cases. diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-absl.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-absl.cpp new file mode 100644 index 0000000000000..b6c67dcb60575 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-absl.cpp @@ -0,0 +1,16 @@ +// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \ +// RUN: -config='{CheckOptions: {performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional"}}' + +#include <string> + +namespace absl { +template <typename T> class optional { +public: + T value_or(T default_value) const; +}; +} // namespace absl + +void positiveAbsl(absl::optional<std::string> opt) { + auto val = opt.value_or("default"); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-rvalue.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-rvalue.cpp new file mode 100644 index 0000000000000..b707921ee9d0a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-rvalue.cpp @@ -0,0 +1,24 @@ +// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \ +// RUN: -config='{CheckOptions: {performance-expensive-value-or.WarnOnRvalueOptional: true}}' + +#include <string> + +namespace std { +template <typename T> class optional { + T val; + bool has; + +public: + optional(); + optional(const optional &); + optional(optional &&); + ~optional(); + T value_or(T default_value) const; +}; +} // namespace std + +std::optional<std::string> getOpt(); +void positiveRvalueOptional() { + auto val = getOpt().value_or("default"); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or.cpp new file mode 100644 index 0000000000000..9bb14dc9c9eb9 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or.cpp @@ -0,0 +1,110 @@ +// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t + +#include <string> +#include <utility> + +namespace std { +template <typename T> class optional { + T val; + bool has; + +public: + optional(); + optional(const optional &); + optional(optional &&); + ~optional(); + T value_or(T default_value) const; +}; +} // namespace std + +void positiveNonTriviallyCopyable(std::optional<std::string> opt) { + auto val = opt.value_or("default"); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} + +struct LargeStruct { + char data[128]; +}; + +void positiveLargeStruct(std::optional<LargeStruct> opt) { + auto val = opt.value_or(LargeStruct{}); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'LargeStruct'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} + +struct SmallNonTrivial { + int x; + SmallNonTrivial(int x) : x(x) {} + SmallNonTrivial(const SmallNonTrivial &o) : x(o.x) {} +}; + +void positiveSmallNonTrivial(std::optional<SmallNonTrivial> opt) { + auto val = opt.value_or(SmallNonTrivial{42}); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'SmallNonTrivial'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} + +void consume(std::string s); +void positiveDirectUse(std::optional<std::string> opt) { + consume(opt.value_or("fallback")); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} + +template <typename T> void positiveTemplate(std::optional<T> opt) { + auto val = opt.value_or(T{}); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} +void instantiate() { + positiveTemplate(std::optional<std::string>{}); +} + +using OptString = std::optional<std::string>; +void positiveTypeAlias(OptString opt) { + auto val = opt.value_or("default"); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} + +void positiveNested(std::optional<std::optional<std::string>> opt) { + auto val = opt.value_or(std::optional<std::string>{}); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::optional<std::basic_string<char>>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} + +void positiveMoveResult(std::optional<std::string> opt) { + auto val = std::move(opt.value_or("default")); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or] +} + +void negativeInt(std::optional<int> opt) { + auto val = opt.value_or(0); +} + +struct SmallPOD { + char x; + char y; +}; + +void negativeSmallPOD(std::optional<SmallPOD> opt) { + auto val = opt.value_or(SmallPOD{0, 0}); +} + +struct EightBytes { + char d[8]; +}; + +void negativeBoundary(std::optional<EightBytes> opt) { + auto val = opt.value_or(EightBytes{}); +} + +std::optional<std::string> getOpt(); +void negativeRvalueOptional() { + auto val = getOpt().value_or("default"); +} + +namespace absl { +template <typename T> class optional { +public: + T value_or(T default_value) const; +}; +} // namespace absl + +void negativeAbslNoConfig(absl::optional<std::string> opt) { + auto val = opt.value_or("default"); +} From a72ba7eb7622486e7d78a95ec92c262a18e70603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <[email protected]> Date: Thu, 28 May 2026 15:41:14 +0200 Subject: [PATCH 2/3] fix local const tidy warnings --- .../clang-tidy/performance/ExpensiveValueOrCheck.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp index aa9e412e99bce..670b6ec67b8b2 100644 --- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp @@ -49,14 +49,14 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) { if (!WarnOnRvalueOptional && ObjExpr && !ObjExpr->isLValue()) return; - QualType ValueType = Call->getType().getCanonicalType(); + const QualType ValueType = Call->getType().getCanonicalType(); if (ValueType->isDependentType() || ValueType->isIncompleteType()) return; const ASTContext &Ctx = *Result.Context; - int64_t ValueSize = Ctx.getTypeSizeInChars(ValueType).getQuantity(); - bool IsExpensive = !ValueType.isTriviallyCopyableType(Ctx) || - ValueSize > static_cast<int64_t>(SizeThreshold); + const int64_t ValueSize = Ctx.getTypeSizeInChars(ValueType).getQuantity(); + const bool IsExpensive = !ValueType.isTriviallyCopyableType(Ctx) || + ValueSize > static_cast<int64_t>(SizeThreshold); if (!IsExpensive) return; From 960b124ece809730d4b18102d6174997deb0053f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <[email protected]> Date: Thu, 28 May 2026 18:51:23 +0200 Subject: [PATCH 3/3] fix RST quoting: use single backticks for default value --- .../docs/clang-tidy/checks/performance/expensive-value-or.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/expensive-value-or.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/expensive-value-or.rst index 0592758a53381..dcfe673898c28 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/performance/expensive-value-or.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/performance/expensive-value-or.rst @@ -44,7 +44,7 @@ Options Semicolon-separated list of fully-qualified names of optional-like class templates to check. The check matches calls to ``value_or`` on - specializations of these templates. Default is ``::std::optional``. + specializations of these templates. Default is `::std::optional`. Example configuration to also check ``absl::optional``: _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
