[clang-tools-extra] [clang-tidy] Add performance-expensive-value-or check (PR #200166)
Endre Fülöp via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 14 15:57:17 PDT 2026
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?= <endre.fulop at sigmatechnology.com>
Date: Thu, 28 May 2026 12:00:10 +0200
Subject: [PATCH 01/26] [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?= <endre.fulop at sigmatechnology.com>
Date: Thu, 28 May 2026 15:41:14 +0200
Subject: [PATCH 02/26] 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?= <endre.fulop at sigmatechnology.com>
Date: Thu, 28 May 2026 18:51:23 +0200
Subject: [PATCH 03/26] 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``:
>From 0ba6f3a7907ba67444a4771106fde3e0433d2c6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Thu, 28 May 2026 18:57:38 +0200
Subject: [PATCH 04/26] fix release notes: synchronize wording with doc opening
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index da0ac85f03eb0..12436e6da69ba 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -278,7 +278,7 @@ New checks
- 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
+ Finds calls to ``value_or`` on optional types where the underlying value
type is expensive to copy.
- New :doc:`performance-string-view-conversions
>From 34686b12aa6562175e8c916b3a28c87f72fee4bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 12:37:52 +0200
Subject: [PATCH 05/26] use type_traits::isExpensiveToCopy
---
.../performance/ExpensiveValueOrCheck.cpp | 11 ++++++-----
.../checkers/performance/expensive-value-or.cpp | 13 +++++++++++++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 670b6ec67b8b2..4b5438fb1fd7a 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -8,6 +8,7 @@
#include "ExpensiveValueOrCheck.h"
#include "../utils/OptionsUtils.h"
+#include "../utils/TypeTraits.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -49,14 +50,14 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
if (!WarnOnRvalueOptional && ObjExpr && !ObjExpr->isLValue())
return;
+ const ASTContext &Ctx = *Result.Context;
const QualType ValueType = Call->getType().getCanonicalType();
- if (ValueType->isDependentType() || ValueType->isIncompleteType())
- return;
+ const bool IsExpensiveType =
+ utils::type_traits::isExpensiveToCopy(ValueType, Ctx).value_or(false);
- const ASTContext &Ctx = *Result.Context;
const int64_t ValueSize = Ctx.getTypeSizeInChars(ValueType).getQuantity();
- const bool IsExpensive = !ValueType.isTriviallyCopyableType(Ctx) ||
- ValueSize > static_cast<int64_t>(SizeThreshold);
+ const bool IsExpensive =
+ IsExpensiveType || ValueSize > static_cast<int64_t>(SizeThreshold);
if (!IsExpensive)
return;
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
index 9bb14dc9c9eb9..3227c9e83ed5c 100644
--- 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
@@ -93,6 +93,19 @@ void negativeBoundary(std::optional<EightBytes> opt) {
auto val = opt.value_or(EightBytes{});
}
+struct TrivialCopyNonTrivialAssign {
+ int x;
+ TrivialCopyNonTrivialAssign() = default;
+ TrivialCopyNonTrivialAssign(const TrivialCopyNonTrivialAssign &) = default;
+ TrivialCopyNonTrivialAssign &operator=(const TrivialCopyNonTrivialAssign &);
+ ~TrivialCopyNonTrivialAssign() = default;
+};
+
+void negativeTrivialCopyNonTrivialAssign(
+ std::optional<TrivialCopyNonTrivialAssign> opt) {
+ auto val = opt.value_or(TrivialCopyNonTrivialAssign{});
+}
+
std::optional<std::string> getOpt();
void negativeRvalueOptional() {
auto val = getOpt().value_or("default");
>From 14461a36cf1dbae9d5df7fc2dcf35f49b580981e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 14:14:30 +0200
Subject: [PATCH 06/26] support alternative spellings of value_or
---
.../performance/ExpensiveValueOrCheck.cpp | 18 ++++++++-----
.../checks/performance/expensive-value-or.rst | 20 +++++++-------
.../performance/expensive-value-or-absl.cpp | 3 +--
.../performance/expensive-value-or.cpp | 27 +++++++++++++++++--
4 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 4b5438fb1fd7a..bfc70e5d572e7 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "ExpensiveValueOrCheck.h"
+#include "../utils/Matchers.h"
#include "../utils/OptionsUtils.h"
#include "../utils/TypeTraits.h"
#include "clang/AST/ASTContext.h"
@@ -22,7 +23,8 @@ ExpensiveValueOrCheck::ExpensiveValueOrCheck(StringRef Name,
SizeThreshold(Options.get("SizeThreshold", 8U)),
WarnOnRvalueOptional(Options.get("WarnOnRvalueOptional", false)),
OptionalTypes(utils::options::parseStringList(
- Options.get("OptionalTypes", "::std::optional"))) {}
+ Options.get("OptionalTypes",
+ "::std::optional;::absl::optional;::boost::optional"))) {}
void ExpensiveValueOrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "SizeThreshold", SizeThreshold);
@@ -32,11 +34,13 @@ void ExpensiveValueOrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
}
void ExpensiveValueOrCheck::registerMatchers(MatchFinder *Finder) {
- auto OptionalTypesMatcher = hasAnyName(OptionalTypes);
+ auto OptionalTypesMatcher =
+ matchers::matchesAnyListedRegexName(OptionalTypes);
+ auto ValueOrMatcher = hasAnyName("value_or", "valueOr", "ValueOr");
Finder->addMatcher(
- cxxMemberCallExpr(callee(cxxMethodDecl(hasName("value_or"),
- ofClass(OptionalTypesMatcher))))
+ cxxMemberCallExpr(
+ callee(cxxMethodDecl(ValueOrMatcher, ofClass(OptionalTypesMatcher))))
.bind("call"),
this);
}
@@ -62,10 +66,12 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
if (!IsExpensive)
return;
+ const CXXMethodDecl *Method = Call->getMethodDecl();
+
diag(Call->getExprLoc(),
- "'value_or' copies expensive type %0; consider using 'operator*' or "
+ "'%0' copies expensive type %1; consider using 'operator*' or "
"'value()' with a separate fallback")
- << ValueType;
+ << Method->getName() << ValueType;
}
} // namespace clang::tidy::performance
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 dcfe673898c28..a9e38a2720a86 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
@@ -3,10 +3,11 @@
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.
+Finds calls to ``value_or`` (and alternative spellings ``valueOr``,
+``ValueOr``) on optional types where the return type is expensive to copy.
+These methods return by value, which involves copying the contained value.
+``value()`` and ``operator*`` return references and can be used to avoid the
+copy when appropriate.
The check is applied to types that are not trivially copyable or whose size
exceeds a configurable threshold. It supports ``std::optional``,
@@ -42,16 +43,17 @@ Options
.. 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`.
+ Semicolon-separated list of regular expressions matching 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;::absl::optional;::boost::optional`.
- Example configuration to also check ``absl::optional``:
+ Example configuration to also check a project-local optional type:
.. code-block:: yaml
CheckOptions:
- performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional"
+ performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::boost::optional;::myproject::.*Optional"
.. option:: WarnOnRvalueOptional
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
index b6c67dcb60575..72a7a4e0577cd 100644
--- 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
@@ -1,5 +1,4 @@
-// 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"}}'
+// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t
#include <string>
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
index 3227c9e83ed5c..6c0dd9efbca3a 100644
--- 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
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t
+// 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;::custom::CamelOptional;::custom::PascalOptional"}}'
#include <string>
#include <utility>
@@ -118,6 +119,28 @@ template <typename T> class optional {
};
} // namespace absl
-void negativeAbslNoConfig(absl::optional<std::string> opt) {
+void positiveAbslDefault(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]
+}
+
+namespace custom {
+template <typename T> class CamelOptional {
+public:
+ T valueOr(T default_value) const;
+};
+template <typename T> class PascalOptional {
+public:
+ T ValueOr(T default_value) const;
+};
+} // namespace custom
+
+void positiveValueOr(custom::CamelOptional<std::string> opt) {
+ auto val = opt.valueOr("default");
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'valueOr' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or]
+}
+
+void positiveValueOrPascal(custom::PascalOptional<std::string> opt) {
+ auto val = opt.ValueOr("default");
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'ValueOr' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or]
}
>From 2a8db054597514b2cf38c66e205c8c17b82ec437 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 15:06:08 +0200
Subject: [PATCH 07/26] make the threshold value 16 by default
---
.../performance/ExpensiveValueOrCheck.cpp | 2 +-
.../checks/performance/expensive-value-or.rst | 2 +-
.../expensive-value-or-size-threshold.cpp | 33 +++++++++++++++++++
.../performance/expensive-value-or.cpp | 8 ++---
4 files changed, 39 insertions(+), 6 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index bfc70e5d572e7..461b3c6608cd1 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -20,7 +20,7 @@ namespace clang::tidy::performance {
ExpensiveValueOrCheck::ExpensiveValueOrCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- SizeThreshold(Options.get("SizeThreshold", 8U)),
+ SizeThreshold(Options.get("SizeThreshold", 16U)),
WarnOnRvalueOptional(Options.get("WarnOnRvalueOptional", false)),
OptionalTypes(utils::options::parseStringList(
Options.get("OptionalTypes",
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 a9e38a2720a86..d41a6864bc4a4 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
@@ -39,7 +39,7 @@ Options
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`.
+ Default is `16`.
.. option:: OptionalTypes
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
new file mode 100644
index 0000000000000..be841d3bcfe0c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \
+// RUN: -config='{CheckOptions: {performance-expensive-value-or.SizeThreshold: 8}}'
+
+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
+
+struct EightBytes {
+ char d[8];
+};
+
+void negativeBoundary(std::optional<EightBytes> opt) {
+ auto val = opt.value_or(EightBytes{});
+}
+
+struct NineBytes {
+ char d[9];
+};
+
+void positiveBoundary(std::optional<NineBytes> opt) {
+ auto val = opt.value_or(NineBytes{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'NineBytes'
+}
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
index 6c0dd9efbca3a..35bb434423b75 100644
--- 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
@@ -86,12 +86,12 @@ void negativeSmallPOD(std::optional<SmallPOD> opt) {
auto val = opt.value_or(SmallPOD{0, 0});
}
-struct EightBytes {
- char d[8];
+struct SixteenBytes {
+ char d[16];
};
-void negativeBoundary(std::optional<EightBytes> opt) {
- auto val = opt.value_or(EightBytes{});
+void negativeBoundary(std::optional<SixteenBytes> opt) {
+ auto val = opt.value_or(SixteenBytes{});
}
struct TrivialCopyNonTrivialAssign {
>From 14be9ebeca2f28667eb8a064c85fba77323f4234 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 15:22:01 +0200
Subject: [PATCH 08/26] factor out std::optional type to mock lib impl
note the we choose not to wire up the other checks test cases yet in
this change. we can do it as a separate NFC commit
---
.../checkers/Inputs/Headers/std/optional | 29 +++++++++++++++++++
.../checkers/Inputs/Headers/std/type_traits | 5 ++++
.../performance/expensive-value-or-rvalue.cpp | 15 +---------
.../expensive-value-or-size-threshold.cpp | 14 +--------
.../performance/expensive-value-or.cpp | 15 +---------
5 files changed, 37 insertions(+), 41 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/optional
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/optional b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/optional
new file mode 100644
index 0000000000000..59772c6e7e125
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/optional
@@ -0,0 +1,29 @@
+#ifndef _CLANG_TIDY_TEST_STD_OPTIONAL_H
+#define _CLANG_TIDY_TEST_STD_OPTIONAL_H
+
+#include <type_traits>
+
+namespace std {
+
+template <typename T> class optional {
+
+// Internal structure of std::optional is not modeled here. If checks need
+// to reason about layout or size of optional itself, add members to match
+// the real implementation for greater precision.
+
+public:
+ optional();
+ optional(const optional &);
+ optional(optional &&);
+ ~optional();
+
+ template <class U = remove_cv_t<T>>
+ constexpr T value_or(U &&default_value) const &;
+
+ template <class U = remove_cv_t<T>>
+ constexpr T value_or(U &&default_value) &&;
+};
+
+} // namespace std
+
+#endif // _CLANG_TIDY_TEST_STD_OPTIONAL_H
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/type_traits b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/type_traits
index e9b6fa76cb6ff..e8f837661eaac 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/type_traits
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/type_traits
@@ -1,3 +1,6 @@
+#ifndef _CLANG_TIDY_TEST_STD_TYPE_TRAITS_H
+#define _CLANG_TIDY_TEST_STD_TYPE_TRAITS_H
+
#include "cstddef"
namespace std {
@@ -449,3 +452,5 @@ struct in_place_t {};
constexpr in_place_t in_place;
} // namespace std
+
+#endif // _CLANG_TIDY_TEST_STD_TYPE_TRAITS_H
\ No newline at end of file
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
index b707921ee9d0a..72b4ed568e470 100644
--- 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
@@ -1,22 +1,9 @@
// 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 <optional>
#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");
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
index be841d3bcfe0c..30426c43b7dac 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
@@ -1,19 +1,7 @@
// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \
// RUN: -config='{CheckOptions: {performance-expensive-value-or.SizeThreshold: 8}}'
-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
+#include <optional>
struct EightBytes {
char d[8];
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
index 35bb434423b75..717b2be59c6d9 100644
--- 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
@@ -1,23 +1,10 @@
// 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;::custom::CamelOptional;::custom::PascalOptional"}}'
+#include <optional>
#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]
>From f247334bf23ccbc3c2dfa3e4c90d6c7e5fe73ab9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 15:35:49 +0200
Subject: [PATCH 09/26] make the rvalue-ness a built-in heuristic, remove
related option
---
.../clang-tidy/performance/ExpensiveValueOrCheck.cpp | 11 ++++++-----
.../clang-tidy/performance/ExpensiveValueOrCheck.h | 1 -
.../checks/performance/expensive-value-or.rst | 8 --------
.../clang-tidy/checkers/Inputs/Headers/std/string | 2 ++
.../performance/expensive-value-or-rvalue.cpp | 11 -----------
.../checkers/performance/expensive-value-or.cpp | 6 ++++++
6 files changed, 14 insertions(+), 25 deletions(-)
delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-rvalue.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 461b3c6608cd1..12c876115c101 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -21,14 +21,12 @@ ExpensiveValueOrCheck::ExpensiveValueOrCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
SizeThreshold(Options.get("SizeThreshold", 16U)),
- WarnOnRvalueOptional(Options.get("WarnOnRvalueOptional", false)),
OptionalTypes(utils::options::parseStringList(
Options.get("OptionalTypes",
"::std::optional;::absl::optional;::boost::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));
}
@@ -51,14 +49,17 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
return;
const Expr *ObjExpr = Call->getImplicitObjectArgument();
- if (!WarnOnRvalueOptional && ObjExpr && !ObjExpr->isLValue())
- return;
const ASTContext &Ctx = *Result.Context;
const QualType ValueType = Call->getType().getCanonicalType();
+
+ // Rvalue optional uses && overload which moves. Suppress if move is cheap.
+ if (ObjExpr && !ObjExpr->isLValue() &&
+ utils::type_traits::hasNonTrivialMoveConstructor(ValueType))
+ return;
+
const bool IsExpensiveType =
utils::type_traits::isExpensiveToCopy(ValueType, Ctx).value_or(false);
-
const int64_t ValueSize = Ctx.getTypeSizeInChars(ValueType).getQuantity();
const bool IsExpensive =
IsExpensiveType || ValueSize > static_cast<int64_t>(SizeThreshold);
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
index 8b46b6d4a9ff0..5a6f22dcdf2a5 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
@@ -31,7 +31,6 @@ class ExpensiveValueOrCheck : public ClangTidyCheck {
private:
const unsigned SizeThreshold;
- const bool WarnOnRvalueOptional;
const std::vector<StringRef> OptionalTypes;
};
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 d41a6864bc4a4..fe38347f4950a 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
@@ -54,11 +54,3 @@ Options
CheckOptions:
performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::boost::optional;::myproject::.*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/Inputs/Headers/std/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string
index dbebeaaa46514..766f240c655fb 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/string
@@ -21,6 +21,8 @@ struct basic_string {
typedef size_t size_type;
typedef basic_string<C, T, A> _Type;
basic_string();
+ basic_string(const basic_string &);
+ basic_string(basic_string &&);
basic_string(basic_string_view<C, T>);
basic_string(const C *p, const A &a = A());
basic_string(const C *p, size_type count);
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
deleted file mode 100644
index 72b4ed568e470..0000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-rvalue.cpp
+++ /dev/null
@@ -1,11 +0,0 @@
-// 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 <optional>
-#include <string>
-
-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
index 717b2be59c6d9..9da82a5ebdeef 100644
--- 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
@@ -99,6 +99,12 @@ void negativeRvalueOptional() {
auto val = getOpt().value_or("default");
}
+std::optional<LargeStruct> getLargeOpt();
+void positiveRvalueNoMoveAdvantage() {
+ auto val = getLargeOpt().value_or(LargeStruct{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'value_or' copies expensive type 'LargeStruct'
+}
+
namespace absl {
template <typename T> class optional {
public:
>From 4cc5fe6b0270688e92036821eac15d99c0a7c124 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 15:48:29 +0200
Subject: [PATCH 10/26] only suggest an alternative if it exists
---
.../performance/ExpensiveValueOrCheck.cpp | 44 +++++++++++++++++--
.../checkers/Inputs/Headers/std/optional | 12 +++++
.../performance/expensive-value-or-absl.cpp | 2 +-
.../performance/expensive-value-or.cpp | 6 +--
4 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 12c876115c101..4b02eb413c56c 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -17,6 +17,44 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
+namespace {
+
+bool hasOperatorStar(const CXXRecordDecl *RD) {
+ return llvm::any_of(RD->methods(), [](const CXXMethodDecl *M) {
+ return M->getOverloadedOperator() == OO_Star;
+ });
+}
+
+StringRef findValueMethod(const CXXRecordDecl *RD) {
+ for (const auto *M : RD->methods()) {
+ if (!M->getDeclName().isIdentifier())
+ continue;
+ StringRef Name = M->getName();
+ if (Name == "value" || Name == "Value")
+ return Name;
+ }
+ return {};
+}
+
+std::string buildSuggestion(const CXXRecordDecl *OptionalClass) {
+ bool HasDeref = hasOperatorStar(OptionalClass);
+ StringRef ValueName = findValueMethod(OptionalClass);
+
+ if (HasDeref && !ValueName.empty())
+ return (llvm::Twine("consider using 'operator*' or '") + ValueName +
+ "()' with a separate fallback")
+ .str();
+ if (HasDeref)
+ return "consider using 'operator*' with a separate fallback";
+ if (!ValueName.empty())
+ return (llvm::Twine("consider using '") + ValueName +
+ "()' with a separate fallback")
+ .str();
+ return "consider avoiding the copy";
+}
+
+} // namespace
+
ExpensiveValueOrCheck::ExpensiveValueOrCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -69,10 +107,8 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
const CXXMethodDecl *Method = Call->getMethodDecl();
- diag(Call->getExprLoc(),
- "'%0' copies expensive type %1; consider using 'operator*' or "
- "'value()' with a separate fallback")
- << Method->getName() << ValueType;
+ diag(Call->getExprLoc(), "'%0' copies expensive type %1; %2")
+ << Method->getName() << ValueType << buildSuggestion(Method->getParent());
}
} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/optional b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/optional
index 59772c6e7e125..b0f134232c26a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/optional
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/optional
@@ -22,6 +22,18 @@ public:
template <class U = remove_cv_t<T>>
constexpr T value_or(U &&default_value) &&;
+
+ const T *operator->() const noexcept;
+ T *operator->() noexcept;
+ const T &operator*() const & noexcept;
+ T &operator*() & noexcept;
+ const T &&operator*() const && noexcept;
+ T &&operator*() && noexcept;
+
+ T &value() &;
+ const T &value() const &;
+ T &&value() &&;
+ const T &&value() const &&;
};
} // namespace std
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
index 72a7a4e0577cd..c36c863d94db4 100644
--- 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
@@ -11,5 +11,5 @@ template <typename T> class optional {
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]
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider avoiding the copy [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
index 9da82a5ebdeef..2ceeede5cb850 100644
--- 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
@@ -114,7 +114,7 @@ template <typename T> class optional {
void positiveAbslDefault(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]
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider avoiding the copy [performance-expensive-value-or]
}
namespace custom {
@@ -130,10 +130,10 @@ template <typename T> class PascalOptional {
void positiveValueOr(custom::CamelOptional<std::string> opt) {
auto val = opt.valueOr("default");
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'valueOr' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or]
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'valueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy [performance-expensive-value-or]
}
void positiveValueOrPascal(custom::PascalOptional<std::string> opt) {
auto val = opt.ValueOr("default");
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'ValueOr' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback [performance-expensive-value-or]
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'ValueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy [performance-expensive-value-or]
}
>From 5e27706923723616a9480b0712e54208e1946e5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 15:57:55 +0200
Subject: [PATCH 11/26] take side effects in the fallback into account
---
.../performance/ExpensiveValueOrCheck.cpp | 10 +++++++++-
.../checkers/performance/expensive-value-or.cpp | 15 +++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 4b02eb413c56c..4e3fde4e0ba2a 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -108,7 +108,15 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
const CXXMethodDecl *Method = Call->getMethodDecl();
diag(Call->getExprLoc(), "'%0' copies expensive type %1; %2")
- << Method->getName() << ValueType << buildSuggestion(Method->getParent());
+ << Method->getName() << ValueType
+ << buildSuggestion(Method->getParent());
+
+ const Expr *FallbackArg = Call->getArg(0)->IgnoreImplicit();
+ if (FallbackArg->HasSideEffects(Ctx))
+ diag(FallbackArg->getExprLoc(),
+ "the fallback is always evaluated; a conditional rewrite would "
+ "change evaluation semantics",
+ DiagnosticIDs::Note);
}
} // namespace clang::tidy::performance
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
index 2ceeede5cb850..fca6cc0099658 100644
--- 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
@@ -36,6 +36,21 @@ void positiveDirectUse(std::optional<std::string> opt) {
// 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]
}
+std::string makeFallback();
+void positiveSideEffectFallback(std::optional<std::string> opt) {
+ auto val = opt.value_or(makeFallback());
+ // 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]
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: note: the fallback is always evaluated; a conditional rewrite would change evaluation semantics
+}
+
+// Constructing a temporary with a non-trivial destructor is treated as a side
+// effect (CXXBindTemporaryExpr), so the note fires here too.
+void positiveSideEffectTemporary(std::optional<std::string> opt) {
+ auto val = opt.value_or(std::string("fallback"));
+ // 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]
+ // CHECK-MESSAGES: :[[@LINE-2]]:27: note: the fallback is always evaluated; a conditional rewrite would change evaluation semantics
+}
+
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]
>From a9f8abff79fabcf0b125173f802c1114e275fb2a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 16:41:48 +0200
Subject: [PATCH 12/26] introduce WarnOnOwnershipTaking to make the reports
actionable
---
.../performance/ExpensiveValueOrCheck.cpp | 33 +++++-
.../performance/ExpensiveValueOrCheck.h | 1 +
.../checks/performance/expensive-value-or.rst | 32 +++++-
.../performance/expensive-value-or-absl.cpp | 3 +-
.../expensive-value-or-size-threshold.cpp | 2 +-
.../performance/expensive-value-or.cpp | 103 +++++++++++-------
6 files changed, 121 insertions(+), 53 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 4e3fde4e0ba2a..7434092d88e2d 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -61,23 +61,45 @@ ExpensiveValueOrCheck::ExpensiveValueOrCheck(StringRef Name,
SizeThreshold(Options.get("SizeThreshold", 16U)),
OptionalTypes(utils::options::parseStringList(
Options.get("OptionalTypes",
- "::std::optional;::absl::optional;::boost::optional"))) {}
+ "::std::optional;::absl::optional;::boost::optional"))),
+ WarnOnOwnershipTaking(Options.get("WarnOnOwnershipTaking", false)) {}
void ExpensiveValueOrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "SizeThreshold", SizeThreshold);
Options.store(Opts, "OptionalTypes",
utils::options::serializeStringList(OptionalTypes));
+ Options.store(Opts, "WarnOnOwnershipTaking", WarnOnOwnershipTaking);
}
void ExpensiveValueOrCheck::registerMatchers(MatchFinder *Finder) {
auto OptionalTypesMatcher =
matchers::matchesAnyListedRegexName(OptionalTypes);
auto ValueOrMatcher = hasAnyName("value_or", "valueOr", "ValueOr");
+ auto ValueOrCall = cxxMemberCallExpr(
+ callee(cxxMethodDecl(ValueOrMatcher, ofClass(OptionalTypesMatcher))));
+ if (WarnOnOwnershipTaking) {
+ Finder->addMatcher(ValueOrCall.bind("call"), this);
+ return;
+ }
+
+ // Binding to const T& variable.
+ Finder->addMatcher(
+ varDecl(hasType(lValueReferenceType(pointee(isConstQualified()))),
+ hasInitializer(ignoringImplicit(ValueOrCall.bind("call")))),
+ this);
+
+ // Passing to a const T& parameter.
+ Finder->addMatcher(callExpr(forEachArgumentWithParam(
+ ignoringImplicit(ValueOrCall.bind("call")),
+ parmVarDecl(hasType(lValueReferenceType(
+ pointee(isConstQualified())))))),
+ this);
+
+ // Calling a const member function on the result.
Finder->addMatcher(
- cxxMemberCallExpr(
- callee(cxxMethodDecl(ValueOrMatcher, ofClass(OptionalTypesMatcher))))
- .bind("call"),
+ cxxMemberCallExpr(on(ignoringImplicit(ValueOrCall.bind("call"))),
+ callee(cxxMethodDecl(isConst()))),
this);
}
@@ -108,8 +130,7 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
const CXXMethodDecl *Method = Call->getMethodDecl();
diag(Call->getExprLoc(), "'%0' copies expensive type %1; %2")
- << Method->getName() << ValueType
- << buildSuggestion(Method->getParent());
+ << Method->getName() << ValueType << buildSuggestion(Method->getParent());
const Expr *FallbackArg = Call->getArg(0)->IgnoreImplicit();
if (FallbackArg->HasSideEffects(Ctx))
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
index 5a6f22dcdf2a5..9a9ed61bf346e 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
@@ -32,6 +32,7 @@ class ExpensiveValueOrCheck : public ClangTidyCheck {
private:
const unsigned SizeThreshold;
const std::vector<StringRef> OptionalTypes;
+ const bool WarnOnOwnershipTaking;
};
} // namespace clang::tidy::performance
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 fe38347f4950a..03237eeb611e8 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
@@ -21,15 +21,29 @@ Example:
#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");
+ void consumeRef(const std::string &);
- // Alternatives that avoid the copy:
- const std::string fallback = "default";
- const auto &ref = opt.has_value() ? *opt : fallback;
+ void example(std::optional<std::string> opt,
+ const std::string &fallback) {
+ // Warning: result binds to const reference, copy is avoidable.
+ const std::string &ref = opt.value_or(fallback);
+
+ // Warning: result passed to const reference parameter.
+ consumeRef(opt.value_or(fallback));
+
+ // Warning: const member called on temporary.
+ auto len = opt.value_or(fallback).size();
+
+ // No warning by default: caller takes ownership.
+ std::string val = opt.value_or("default");
}
+By default, the check only warns in reference-friendly contexts where the copy
+is clearly avoidable: binding to ``const T&``, passing to a ``const T&``
+parameter, or calling a const member function on the temporary. Contexts where
+the caller takes ownership (binding to a value, passing to a by-value
+parameter) are not flagged unless ``WarnOnOwnershipTaking`` is enabled.
+
Options
-------
@@ -54,3 +68,9 @@ Options
CheckOptions:
performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::boost::optional;::myproject::.*Optional"
+
+.. option:: WarnOnOwnershipTaking
+
+ When `true`, the check also warns when the result of ``value_or`` is used in
+ an ownership-taking context (e.g., initializing a value variable or passing
+ to a by-value parameter). Default is `false`.
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
index c36c863d94db4..4bd0c0c949b3e 100644
--- 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
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t
+// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \
+// RUN: -config='{CheckOptions: {performance-expensive-value-or.WarnOnOwnershipTaking: true}}'
#include <string>
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
index 30426c43b7dac..ded7b64390e17 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
@@ -1,5 +1,5 @@
// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \
-// RUN: -config='{CheckOptions: {performance-expensive-value-or.SizeThreshold: 8}}'
+// RUN: -config='{CheckOptions: {performance-expensive-value-or.SizeThreshold: 8, performance-expensive-value-or.WarnOnOwnershipTaking: true}}'
#include <optional>
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
index fca6cc0099658..12b7d9cbf395c 100644
--- 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
@@ -1,22 +1,52 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \
+// RUN: %check_clang_tidy -std=c++17-or-later -check-suffix=DEFAULT %s performance-expensive-value-or %t \
// RUN: -config='{CheckOptions: {performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional"}}'
+// RUN: %check_clang_tidy -std=c++17-or-later -check-suffix=AGGRESSIVE %s performance-expensive-value-or %t \
+// RUN: -config='{CheckOptions: {performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional", performance-expensive-value-or.WarnOnOwnershipTaking: true}}'
#include <optional>
#include <string>
#include <utility>
-void positiveNonTriviallyCopyable(std::optional<std::string> opt) {
+void consumeRef(const std::string &);
+void consume(std::string s);
+
+// Reference-friendly contexts: warn in both default and aggressive modes.
+
+void positiveConstRefBinding(std::optional<std::string> opt,
+ const std::string &fallback) {
+ const std::string &ref = opt.value_or(fallback);
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+}
+
+void positiveConstRefParam(std::optional<std::string> opt,
+ const std::string &fallback) {
+ consumeRef(opt.value_or(fallback));
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+}
+
+void positiveConstMemberCall(std::optional<std::string> opt,
+ const std::string &fallback) {
+ auto len = opt.value_or(fallback).size();
+ // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+}
+
+// Ownership-taking contexts: only warn in aggressive mode.
+
+void positiveOwnershipValue(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]
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
}
struct LargeStruct {
char data[128];
};
-void positiveLargeStruct(std::optional<LargeStruct> opt) {
+void positiveOwnershipLarge(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]
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'LargeStruct'
}
struct SmallNonTrivial {
@@ -25,56 +55,58 @@ struct SmallNonTrivial {
SmallNonTrivial(const SmallNonTrivial &o) : x(o.x) {}
};
-void positiveSmallNonTrivial(std::optional<SmallNonTrivial> opt) {
+void positiveOwnershipSmallNonTrivial(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]
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'SmallNonTrivial'
}
-void consume(std::string s);
-void positiveDirectUse(std::optional<std::string> opt) {
+void positiveOwnershipByValueParam(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]
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:15: warning: 'value_or' copies expensive type 'std::basic_string<char>'
}
+// Side-effect note tests (aggressive mode).
+
std::string makeFallback();
void positiveSideEffectFallback(std::optional<std::string> opt) {
auto val = opt.value_or(makeFallback());
- // 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]
- // CHECK-MESSAGES: :[[@LINE-2]]:27: note: the fallback is always evaluated; a conditional rewrite would change evaluation semantics
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:27: note: the fallback is always evaluated
}
// Constructing a temporary with a non-trivial destructor is treated as a side
// effect (CXXBindTemporaryExpr), so the note fires here too.
void positiveSideEffectTemporary(std::optional<std::string> opt) {
auto val = opt.value_or(std::string("fallback"));
- // 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]
- // CHECK-MESSAGES: :[[@LINE-2]]:27: note: the fallback is always evaluated; a conditional rewrite would change evaluation semantics
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:27: note: the fallback is always evaluated
}
+// Template instantiation (aggressive mode).
+
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]
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
}
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]
-}
+// Rvalue optional tests.
-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]
+std::optional<std::string> getOpt();
+void negativeRvalueOptional() {
+ auto val = getOpt().value_or("default");
}
-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]
+std::optional<LargeStruct> getLargeOpt();
+void positiveRvalueNoMoveAdvantage() {
+ auto val = getLargeOpt().value_or(LargeStruct{});
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:28: warning: 'value_or' copies expensive type 'LargeStruct'
}
+// Negative cases: no warning in either mode.
+
void negativeInt(std::optional<int> opt) {
auto val = opt.value_or(0);
}
@@ -96,6 +128,8 @@ void negativeBoundary(std::optional<SixteenBytes> opt) {
auto val = opt.value_or(SixteenBytes{});
}
+// Non-trivially-copyable (due to user-defined copy assignment) but copy
+// construction itself is trivial. No point warning about an inexpensive copy.
struct TrivialCopyNonTrivialAssign {
int x;
TrivialCopyNonTrivialAssign() = default;
@@ -109,16 +143,7 @@ void negativeTrivialCopyNonTrivialAssign(
auto val = opt.value_or(TrivialCopyNonTrivialAssign{});
}
-std::optional<std::string> getOpt();
-void negativeRvalueOptional() {
- auto val = getOpt().value_or("default");
-}
-
-std::optional<LargeStruct> getLargeOpt();
-void positiveRvalueNoMoveAdvantage() {
- auto val = getLargeOpt().value_or(LargeStruct{});
- // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'value_or' copies expensive type 'LargeStruct'
-}
+// Alternative spellings and custom optional types (aggressive mode).
namespace absl {
template <typename T> class optional {
@@ -129,7 +154,7 @@ template <typename T> class optional {
void positiveAbslDefault(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 avoiding the copy [performance-expensive-value-or]
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
}
namespace custom {
@@ -145,10 +170,10 @@ template <typename T> class PascalOptional {
void positiveValueOr(custom::CamelOptional<std::string> opt) {
auto val = opt.valueOr("default");
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'valueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy [performance-expensive-value-or]
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'valueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
}
void positiveValueOrPascal(custom::PascalOptional<std::string> opt) {
auto val = opt.ValueOr("default");
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'ValueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy [performance-expensive-value-or]
+ // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'ValueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
}
>From 022f0001ee230ba63ecb48c502527d1b868d538e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 17:12:49 +0200
Subject: [PATCH 13/26] add fixit generation for the reference-friendly and
non-side effect case
---
.../performance/ExpensiveValueOrCheck.cpp | 47 +++++++++++++++++--
.../docs/clang-tidy/checks/list.rst | 2 +-
.../performance/expensive-value-or.cpp | 11 +++--
3 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 7434092d88e2d..1b2bf583b63c0 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -12,6 +12,7 @@
#include "../utils/TypeTraits.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
@@ -53,6 +54,31 @@ std::string buildSuggestion(const CXXRecordDecl *OptionalClass) {
return "consider avoiding the copy";
}
+std::optional<FixItHint>
+buildFixIt(const CXXMemberCallExpr *Call, const Expr *ObjExpr,
+ const Expr *FallbackArg, const CXXRecordDecl *OptionalClass,
+ const ASTContext &Ctx, const SourceManager &SM,
+ const LangOptions &LO) {
+ if (!ObjExpr || !ObjExpr->isLValue())
+ return std::nullopt;
+ if (FallbackArg->HasSideEffects(Ctx))
+ return std::nullopt;
+ if (!hasOperatorStar(OptionalClass))
+ return std::nullopt;
+
+ StringRef ObjText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(ObjExpr->getSourceRange()), SM, LO);
+ StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(FallbackArg->getSourceRange()), SM, LO);
+
+ if (ObjText.empty() || ArgText.empty())
+ return std::nullopt;
+
+ std::string Replacement =
+ ("(" + ObjText + " ? *" + ObjText + " : " + ArgText + ")").str();
+ return FixItHint::CreateReplacement(Call->getSourceRange(), Replacement);
+}
+
} // namespace
ExpensiveValueOrCheck::ExpensiveValueOrCheck(StringRef Name,
@@ -128,12 +154,23 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
return;
const CXXMethodDecl *Method = Call->getMethodDecl();
-
- diag(Call->getExprLoc(), "'%0' copies expensive type %1; %2")
- << Method->getName() << ValueType << buildSuggestion(Method->getParent());
-
+ const CXXRecordDecl *OptionalClass = Method->getParent();
const Expr *FallbackArg = Call->getArg(0)->IgnoreImplicit();
- if (FallbackArg->HasSideEffects(Ctx))
+ const bool HasSideEffects = FallbackArg->HasSideEffects(Ctx);
+
+ {
+ auto Diag = diag(Call->getExprLoc(), "'%0' copies expensive type %1; %2")
+ << Method->getName() << ValueType
+ << buildSuggestion(OptionalClass);
+
+ if (!HasSideEffects) {
+ if (auto Fix = buildFixIt(Call, ObjExpr, FallbackArg, OptionalClass, Ctx,
+ *Result.SourceManager, Ctx.getLangOpts()))
+ Diag << *Fix;
+ }
+ }
+
+ if (HasSideEffects)
diag(FallbackArg->getExprLoc(),
"the fallback is always evaluated; a conditional rewrite would "
"change evaluation semantics",
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 974655cff268c..060af05c5f4aa 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -352,7 +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-expensive-value-or <performance/expensive-value-or>`, "Yes"
: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/test/clang-tidy/checkers/performance/expensive-value-or.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or.cpp
index 12b7d9cbf395c..802029cd76453 100644
--- 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
@@ -17,6 +17,8 @@ void positiveConstRefBinding(std::optional<std::string> opt,
const std::string &ref = opt.value_or(fallback);
// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'
// CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-FIXES-DEFAULT: const std::string &ref = (opt ? *opt : fallback);
+ // CHECK-FIXES-AGGRESSIVE: const std::string &ref = (opt ? *opt : fallback);
}
void positiveConstRefParam(std::optional<std::string> opt,
@@ -24,6 +26,8 @@ void positiveConstRefParam(std::optional<std::string> opt,
consumeRef(opt.value_or(fallback));
// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
// CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-FIXES-DEFAULT: consumeRef((opt ? *opt : fallback));
+ // CHECK-FIXES-AGGRESSIVE: consumeRef((opt ? *opt : fallback));
}
void positiveConstMemberCall(std::optional<std::string> opt,
@@ -31,6 +35,8 @@ void positiveConstMemberCall(std::optional<std::string> opt,
auto len = opt.value_or(fallback).size();
// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
// CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-FIXES-DEFAULT: auto len = (opt ? *opt : fallback).size();
+ // CHECK-FIXES-AGGRESSIVE: auto len = (opt ? *opt : fallback).size();
}
// Ownership-taking contexts: only warn in aggressive mode.
@@ -74,12 +80,11 @@ void positiveSideEffectFallback(std::optional<std::string> opt) {
// CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:27: note: the fallback is always evaluated
}
-// Constructing a temporary with a non-trivial destructor is treated as a side
-// effect (CXXBindTemporaryExpr), so the note fires here too.
+// Constructing a temporary with a non-trivial destructor is not treated as a
+// definite side effect, so no note fires here.
void positiveSideEffectTemporary(std::optional<std::string> opt) {
auto val = opt.value_or(std::string("fallback"));
// CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:27: note: the fallback is always evaluated
}
// Template instantiation (aggressive mode).
>From d8b16d79b1e2e817561c87f3680b55e1f4850fa8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 17:13:24 +0200
Subject: [PATCH 14/26] remove redundant nullptr check
---
.../clang-tidy/performance/ExpensiveValueOrCheck.cpp | 4 ----
1 file changed, 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 1b2bf583b63c0..5b170e0ef1a2a 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -131,11 +131,7 @@ void ExpensiveValueOrCheck::registerMatchers(MatchFinder *Finder) {
void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
- if (!Call)
- return;
-
const Expr *ObjExpr = Call->getImplicitObjectArgument();
-
const ASTContext &Ctx = *Result.Context;
const QualType ValueType = Call->getType().getCanonicalType();
>From b768dc4359ca182a35d3b33a0dd1bea666526514 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 17:20:55 +0200
Subject: [PATCH 15/26] remove redundant absl test file
---
.../performance/expensive-value-or-absl.cpp | 16 ----------------
1 file changed, 16 deletions(-)
delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-absl.cpp
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
deleted file mode 100644
index 4bd0c0c949b3e..0000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-absl.cpp
+++ /dev/null
@@ -1,16 +0,0 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \
-// RUN: -config='{CheckOptions: {performance-expensive-value-or.WarnOnOwnershipTaking: true}}'
-
-#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 avoiding the copy [performance-expensive-value-or]
-}
>From fdd03e5e4e4f907cfe2e63837b67972be4809e3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 17:43:55 +0200
Subject: [PATCH 16/26] orgainize tests and check fixit generation
---
.../expensive-value-or-size-threshold.cpp | 7 +-
.../performance/expensive-value-or.cpp | 74 ++++++++++---------
2 files changed, 45 insertions(+), 36 deletions(-)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
index ded7b64390e17..28b2b725ef762 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
@@ -1,5 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11-or-later %s performance-expensive-value-or %t \
-// RUN: -config='{CheckOptions: {performance-expensive-value-or.SizeThreshold: 8, performance-expensive-value-or.WarnOnOwnershipTaking: true}}'
+// RUN: %check_clang_tidy -std=c++17-or-later %s performance-expensive-value-or %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: performance-expensive-value-or.SizeThreshold: 8, \
+// RUN: performance-expensive-value-or.WarnOnOwnershipTaking: true \
+// RUN: }}'
#include <optional>
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
index 802029cd76453..17ef77b034b16 100644
--- 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
@@ -1,49 +1,55 @@
-// RUN: %check_clang_tidy -std=c++17-or-later -check-suffix=DEFAULT %s performance-expensive-value-or %t \
-// RUN: -config='{CheckOptions: {performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional"}}'
-// RUN: %check_clang_tidy -std=c++17-or-later -check-suffix=AGGRESSIVE %s performance-expensive-value-or %t \
-// RUN: -config='{CheckOptions: {performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional", performance-expensive-value-or.WarnOnOwnershipTaking: true}}'
+// RUN: %check_clang_tidy -std=c++17-or-later -check-suffixes=NON-OWNING %s \
+// RUN: performance-expensive-value-or %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional" \
+// RUN: }}'
+// RUN: %check_clang_tidy -std=c++17-or-later -check-suffixes=OWNING %s \
+// RUN: performance-expensive-value-or %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional", \
+// RUN: performance-expensive-value-or.WarnOnOwnershipTaking: true \
+// RUN: }}'
#include <optional>
#include <string>
-#include <utility>
void consumeRef(const std::string &);
void consume(std::string s);
-// Reference-friendly contexts: warn in both default and aggressive modes.
+// Reference-friendly contexts: warn in both modes.
void positiveConstRefBinding(std::optional<std::string> opt,
const std::string &fallback) {
const std::string &ref = opt.value_or(fallback);
- // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'
- // CHECK-FIXES-DEFAULT: const std::string &ref = (opt ? *opt : fallback);
- // CHECK-FIXES-AGGRESSIVE: const std::string &ref = (opt ? *opt : fallback);
+ // CHECK-MESSAGES-NON-OWNING: :[[@LINE-1]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-2]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-FIXES-NON-OWNING: const std::string &ref = (opt ? *opt : fallback);
+ // CHECK-FIXES-OWNING: const std::string &ref = (opt ? *opt : fallback);
}
void positiveConstRefParam(std::optional<std::string> opt,
const std::string &fallback) {
consumeRef(opt.value_or(fallback));
- // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
- // CHECK-FIXES-DEFAULT: consumeRef((opt ? *opt : fallback));
- // CHECK-FIXES-AGGRESSIVE: consumeRef((opt ? *opt : fallback));
+ // CHECK-MESSAGES-NON-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-2]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-FIXES-NON-OWNING: consumeRef((opt ? *opt : fallback));
+ // CHECK-FIXES-OWNING: consumeRef((opt ? *opt : fallback));
}
void positiveConstMemberCall(std::optional<std::string> opt,
const std::string &fallback) {
auto len = opt.value_or(fallback).size();
- // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
- // CHECK-FIXES-DEFAULT: auto len = (opt ? *opt : fallback).size();
- // CHECK-FIXES-AGGRESSIVE: auto len = (opt ? *opt : fallback).size();
+ // CHECK-MESSAGES-NON-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-2]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-FIXES-NON-OWNING: auto len = (opt ? *opt : fallback).size();
+ // CHECK-FIXES-OWNING: auto len = (opt ? *opt : fallback).size();
}
-// Ownership-taking contexts: only warn in aggressive mode.
+// Ownership-taking contexts: only warn in OWNING mode.
void positiveOwnershipValue(std::optional<std::string> opt) {
auto val = opt.value_or("default");
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
}
struct LargeStruct {
@@ -52,7 +58,7 @@ struct LargeStruct {
void positiveOwnershipLarge(std::optional<LargeStruct> opt) {
auto val = opt.value_or(LargeStruct{});
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'LargeStruct'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'LargeStruct'
}
struct SmallNonTrivial {
@@ -63,35 +69,35 @@ struct SmallNonTrivial {
void positiveOwnershipSmallNonTrivial(std::optional<SmallNonTrivial> opt) {
auto val = opt.value_or(SmallNonTrivial{42});
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'SmallNonTrivial'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'SmallNonTrivial'
}
void positiveOwnershipByValueParam(std::optional<std::string> opt) {
consume(opt.value_or("fallback"));
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:15: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:15: warning: 'value_or' copies expensive type 'std::basic_string<char>'
}
-// Side-effect note tests (aggressive mode).
+// Side-effect note tests.
std::string makeFallback();
void positiveSideEffectFallback(std::optional<std::string> opt) {
auto val = opt.value_or(makeFallback());
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-2]]:27: note: the fallback is always evaluated
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-2]]:27: note: the fallback is always evaluated
}
// Constructing a temporary with a non-trivial destructor is not treated as a
// definite side effect, so no note fires here.
void positiveSideEffectTemporary(std::optional<std::string> opt) {
auto val = opt.value_or(std::string("fallback"));
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
}
-// Template instantiation (aggressive mode).
+// Template instantiation.
template <typename T> void positiveTemplate(std::optional<T> opt) {
auto val = opt.value_or(T{});
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'
}
void instantiate() {
positiveTemplate(std::optional<std::string>{});
@@ -107,7 +113,7 @@ void negativeRvalueOptional() {
std::optional<LargeStruct> getLargeOpt();
void positiveRvalueNoMoveAdvantage() {
auto val = getLargeOpt().value_or(LargeStruct{});
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:28: warning: 'value_or' copies expensive type 'LargeStruct'
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:28: warning: 'value_or' copies expensive type 'LargeStruct'
}
// Negative cases: no warning in either mode.
@@ -148,7 +154,7 @@ void negativeTrivialCopyNonTrivialAssign(
auto val = opt.value_or(TrivialCopyNonTrivialAssign{});
}
-// Alternative spellings and custom optional types (aggressive mode).
+// Alternative spellings and custom optional types.
namespace absl {
template <typename T> class optional {
@@ -159,7 +165,7 @@ template <typename T> class optional {
void positiveAbslDefault(absl::optional<std::string> opt) {
auto val = opt.value_or("default");
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
}
namespace custom {
@@ -175,10 +181,10 @@ template <typename T> class PascalOptional {
void positiveValueOr(custom::CamelOptional<std::string> opt) {
auto val = opt.valueOr("default");
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'valueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'valueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
}
void positiveValueOrPascal(custom::PascalOptional<std::string> opt) {
auto val = opt.ValueOr("default");
- // CHECK-MESSAGES-AGGRESSIVE: :[[@LINE-1]]:18: warning: 'ValueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'ValueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
}
>From a7dd03551389cfec70cb5f384b31dcb4945c913c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 17:48:58 +0200
Subject: [PATCH 17/26] no fixits for macro usage
---
.../clang-tidy/performance/ExpensiveValueOrCheck.cpp | 2 ++
.../checkers/performance/expensive-value-or.cpp | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 5b170e0ef1a2a..206ed27b467b7 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -59,6 +59,8 @@ buildFixIt(const CXXMemberCallExpr *Call, const Expr *ObjExpr,
const Expr *FallbackArg, const CXXRecordDecl *OptionalClass,
const ASTContext &Ctx, const SourceManager &SM,
const LangOptions &LO) {
+ if (Call->getBeginLoc().isMacroID())
+ return std::nullopt;
if (!ObjExpr || !ObjExpr->isLValue())
return std::nullopt;
if (FallbackArg->HasSideEffects(Ctx))
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
index 17ef77b034b16..fc16b6e0d7bf3 100644
--- 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
@@ -188,3 +188,12 @@ void positiveValueOrPascal(custom::PascalOptional<std::string> opt) {
auto val = opt.ValueOr("default");
// CHECK-MESSAGES-OWNING: :[[@LINE-1]]:18: warning: 'ValueOr' copies expensive type 'std::basic_string<char>'; consider avoiding the copy
}
+
+// Macro test: verify the warning points to a useful location.
+
+#define GET_OR_DEFAULT(opt, def) opt.value_or(def)
+
+void positiveMacro(std::optional<std::string> opt) {
+ auto val = GET_OR_DEFAULT(opt, "default");
+ // CHECK-MESSAGES-OWNING: :[[@LINE-1]]:14: warning: 'value_or' copies expensive type 'std::basic_string<char>'
+}
>From 5cb6845974a7c23022d230d7e5c504684134077d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 17:59:23 +0200
Subject: [PATCH 18/26] clean up preconditions, minimal logic checks
---
.../performance/ExpensiveValueOrCheck.cpp | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 206ed27b467b7..55e975a5a91c4 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -57,13 +57,10 @@ std::string buildSuggestion(const CXXRecordDecl *OptionalClass) {
std::optional<FixItHint>
buildFixIt(const CXXMemberCallExpr *Call, const Expr *ObjExpr,
const Expr *FallbackArg, const CXXRecordDecl *OptionalClass,
- const ASTContext &Ctx, const SourceManager &SM,
- const LangOptions &LO) {
+ const SourceManager &SM, const LangOptions &LO) {
if (Call->getBeginLoc().isMacroID())
return std::nullopt;
- if (!ObjExpr || !ObjExpr->isLValue())
- return std::nullopt;
- if (FallbackArg->HasSideEffects(Ctx))
+ if (!ObjExpr->isLValue())
return std::nullopt;
if (!hasOperatorStar(OptionalClass))
return std::nullopt;
@@ -133,12 +130,15 @@ void ExpensiveValueOrCheck::registerMatchers(MatchFinder *Finder) {
void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+ assert(Call && "Matcher guaranteed a bound 'call' node");
const Expr *ObjExpr = Call->getImplicitObjectArgument();
+ assert(ObjExpr && "CXXMemberCallExpr must have an implicit object argument");
+
const ASTContext &Ctx = *Result.Context;
- const QualType ValueType = Call->getType().getCanonicalType();
+ const QualType ValueType = Call->getType();
// Rvalue optional uses && overload which moves. Suppress if move is cheap.
- if (ObjExpr && !ObjExpr->isLValue() &&
+ if (!ObjExpr->isLValue() &&
utils::type_traits::hasNonTrivialMoveConstructor(ValueType))
return;
@@ -162,8 +162,8 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
<< buildSuggestion(OptionalClass);
if (!HasSideEffects) {
- if (auto Fix = buildFixIt(Call, ObjExpr, FallbackArg, OptionalClass, Ctx,
- *Result.SourceManager, Ctx.getLangOpts()))
+ if (auto Fix = buildFixIt(Call, ObjExpr, FallbackArg, OptionalClass,
+ Ctx.getSourceManager(), Ctx.getLangOpts()))
Diag << *Fix;
}
}
>From b12d39e72f141891ba7eaf00e56b6cc28d14bcfb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 18:20:10 +0200
Subject: [PATCH 19/26] generic cpp is the new language support
---
.../clang-tidy/performance/ExpensiveValueOrCheck.h | 2 +-
.../performance/expensive-value-or-size-threshold.cpp | 2 +-
.../clang-tidy/checkers/performance/expensive-value-or.cpp | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
index 9a9ed61bf346e..e5915c45eaf08 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
@@ -23,7 +23,7 @@ class ExpensiveValueOrCheck : public ClangTidyCheck {
public:
ExpensiveValueOrCheck(StringRef Name, ClangTidyContext *Context);
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
- return LangOpts.CPlusPlus11;
+ return LangOpts.CPlusPlus;
}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
index 28b2b725ef762..f69d3dfbfecdc 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/expensive-value-or-size-threshold.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++17-or-later %s performance-expensive-value-or %t -- \
+// RUN: %check_clang_tidy %s performance-expensive-value-or %t -- \
// RUN: -config='{CheckOptions: { \
// RUN: performance-expensive-value-or.SizeThreshold: 8, \
// RUN: performance-expensive-value-or.WarnOnOwnershipTaking: true \
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
index fc16b6e0d7bf3..c06467676ff4c 100644
--- 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
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy -std=c++17-or-later -check-suffixes=NON-OWNING %s \
+// RUN: %check_clang_tidy -check-suffixes=NON-OWNING %s \
// RUN: performance-expensive-value-or %t -- \
// RUN: -config='{CheckOptions: { \
// RUN: performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional" \
// RUN: }}'
-// RUN: %check_clang_tidy -std=c++17-or-later -check-suffixes=OWNING %s \
+// RUN: %check_clang_tidy -check-suffixes=OWNING %s \
// RUN: performance-expensive-value-or %t -- \
// RUN: -config='{CheckOptions: { \
// RUN: performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional", \
>From e8336be445c83daa8152f681d204de480f9cd430 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Mon, 1 Jun 2026 18:35:47 +0200
Subject: [PATCH 20/26] fix tidy lint warnings
---
.../performance/ExpensiveValueOrCheck.cpp | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 55e975a5a91c4..3cdddaade7a72 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -18,15 +18,13 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
-namespace {
-
-bool hasOperatorStar(const CXXRecordDecl *RD) {
+static bool hasOperatorStar(const CXXRecordDecl *RD) {
return llvm::any_of(RD->methods(), [](const CXXMethodDecl *M) {
return M->getOverloadedOperator() == OO_Star;
});
}
-StringRef findValueMethod(const CXXRecordDecl *RD) {
+static StringRef findValueMethod(const CXXRecordDecl *RD) {
for (const auto *M : RD->methods()) {
if (!M->getDeclName().isIdentifier())
continue;
@@ -37,8 +35,8 @@ StringRef findValueMethod(const CXXRecordDecl *RD) {
return {};
}
-std::string buildSuggestion(const CXXRecordDecl *OptionalClass) {
- bool HasDeref = hasOperatorStar(OptionalClass);
+static std::string buildSuggestion(const CXXRecordDecl *OptionalClass) {
+ const bool HasDeref = hasOperatorStar(OptionalClass);
StringRef ValueName = findValueMethod(OptionalClass);
if (HasDeref && !ValueName.empty())
@@ -54,7 +52,7 @@ std::string buildSuggestion(const CXXRecordDecl *OptionalClass) {
return "consider avoiding the copy";
}
-std::optional<FixItHint>
+static std::optional<FixItHint>
buildFixIt(const CXXMemberCallExpr *Call, const Expr *ObjExpr,
const Expr *FallbackArg, const CXXRecordDecl *OptionalClass,
const SourceManager &SM, const LangOptions &LO) {
@@ -73,13 +71,11 @@ buildFixIt(const CXXMemberCallExpr *Call, const Expr *ObjExpr,
if (ObjText.empty() || ArgText.empty())
return std::nullopt;
- std::string Replacement =
+ const std::string Replacement =
("(" + ObjText + " ? *" + ObjText + " : " + ArgText + ")").str();
return FixItHint::CreateReplacement(Call->getSourceRange(), Replacement);
}
-} // namespace
-
ExpensiveValueOrCheck::ExpensiveValueOrCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
>From e9d69d91e2be92c6bb22ecbee7f589bf0dd25b6e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Sun, 7 Jun 2026 22:44:54 +0200
Subject: [PATCH 21/26] remove yaml config example, fix option reference
---
.../clang-tidy/checks/performance/expensive-value-or.rst | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
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 03237eeb611e8..16d839d81c9d0 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
@@ -42,7 +42,7 @@ By default, the check only warns in reference-friendly contexts where the copy
is clearly avoidable: binding to ``const T&``, passing to a ``const T&``
parameter, or calling a const member function on the temporary. Contexts where
the caller takes ownership (binding to a value, passing to a by-value
-parameter) are not flagged unless ``WarnOnOwnershipTaking`` is enabled.
+parameter) are not flagged unless :option:`WarnOnOwnershipTaking` is enabled.
Options
-------
@@ -62,13 +62,6 @@ Options
``value_or`` on specializations of these templates. Default is
`::std::optional;::absl::optional;::boost::optional`.
- Example configuration to also check a project-local optional type:
-
- .. code-block:: yaml
-
- CheckOptions:
- performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::boost::optional;::myproject::.*Optional"
-
.. option:: WarnOnOwnershipTaking
When `true`, the check also warns when the result of ``value_or`` is used in
>From 7ea80d6ee2cf0aa05fae8c870fee3b0d2c81c865 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Fri, 10 Jul 2026 14:17:46 +0200
Subject: [PATCH 22/26] add newline at EOF in std/type_traits test header
---
.../test/clang-tidy/checkers/Inputs/Headers/std/type_traits | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/type_traits b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/type_traits
index e8f837661eaac..9ce80591f857d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/type_traits
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/type_traits
@@ -453,4 +453,4 @@ constexpr in_place_t in_place;
} // namespace std
-#endif // _CLANG_TIDY_TEST_STD_TYPE_TRAITS_H
\ No newline at end of file
+#endif // _CLANG_TIDY_TEST_STD_TYPE_TRAITS_H
>From 1e61efc04f1777efec424142c2ed19c3dfe99992 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Fri, 10 Jul 2026 14:21:09 +0200
Subject: [PATCH 23/26] document OptionalTypes member as regex patterns
---
.../clang-tidy/performance/ExpensiveValueOrCheck.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
index e5915c45eaf08..fc1afa58c096a 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.h
@@ -31,6 +31,8 @@ class ExpensiveValueOrCheck : public ClangTidyCheck {
private:
const unsigned SizeThreshold;
+ /// Semicolon-separated regex patterns matching fully-qualified optional
+ /// types.
const std::vector<StringRef> OptionalTypes;
const bool WarnOnOwnershipTaking;
};
>From e2954ab77a64297f8f1142873538984d3ac3a801 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Fri, 10 Jul 2026 14:23:23 +0200
Subject: [PATCH 24/26] use tooling::fixit::getText instead of manual
Lexer::getSourceText
---
.../clang-tidy/performance/CMakeLists.txt | 1 +
.../performance/ExpensiveValueOrCheck.cpp | 20 +++++++++----------
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 913421996f505..f55a6cf2800f3 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -45,4 +45,5 @@ clang_target_link_libraries(clangTidyPerformanceModule
clangAnalysis
clangBasic
clangLex
+ clangTooling
)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 3cdddaade7a72..3bc1382168802 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -12,7 +12,7 @@
#include "../utils/TypeTraits.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
using namespace clang::ast_matchers;
@@ -52,10 +52,11 @@ static std::string buildSuggestion(const CXXRecordDecl *OptionalClass) {
return "consider avoiding the copy";
}
-static std::optional<FixItHint>
-buildFixIt(const CXXMemberCallExpr *Call, const Expr *ObjExpr,
- const Expr *FallbackArg, const CXXRecordDecl *OptionalClass,
- const SourceManager &SM, const LangOptions &LO) {
+static std::optional<FixItHint> buildFixIt(const CXXMemberCallExpr *Call,
+ const Expr *ObjExpr,
+ const Expr *FallbackArg,
+ const CXXRecordDecl *OptionalClass,
+ const ASTContext &Ctx) {
if (Call->getBeginLoc().isMacroID())
return std::nullopt;
if (!ObjExpr->isLValue())
@@ -63,10 +64,8 @@ buildFixIt(const CXXMemberCallExpr *Call, const Expr *ObjExpr,
if (!hasOperatorStar(OptionalClass))
return std::nullopt;
- StringRef ObjText = Lexer::getSourceText(
- CharSourceRange::getTokenRange(ObjExpr->getSourceRange()), SM, LO);
- StringRef ArgText = Lexer::getSourceText(
- CharSourceRange::getTokenRange(FallbackArg->getSourceRange()), SM, LO);
+ StringRef ObjText = tooling::fixit::getText(*ObjExpr, Ctx);
+ StringRef ArgText = tooling::fixit::getText(*FallbackArg, Ctx);
if (ObjText.empty() || ArgText.empty())
return std::nullopt;
@@ -158,8 +157,7 @@ void ExpensiveValueOrCheck::check(const MatchFinder::MatchResult &Result) {
<< buildSuggestion(OptionalClass);
if (!HasSideEffects) {
- if (auto Fix = buildFixIt(Call, ObjExpr, FallbackArg, OptionalClass,
- Ctx.getSourceManager(), Ctx.getLangOpts()))
+ if (auto Fix = buildFixIt(Call, ObjExpr, FallbackArg, OptionalClass, Ctx))
Diag << *Fix;
}
}
>From 63c6a48321f0a93c98365c8682e03821077b5044 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Fri, 10 Jul 2026 14:31:31 +0200
Subject: [PATCH 25/26] use DeclContext::lookup to find inherited methods
---
.../performance/ExpensiveValueOrCheck.cpp | 15 ++++----
.../performance/expensive-value-or.cpp | 36 +++++++++++++++++--
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index 3bc1382168802..a744c6656a76b 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -19,17 +19,16 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
static bool hasOperatorStar(const CXXRecordDecl *RD) {
- return llvm::any_of(RD->methods(), [](const CXXMethodDecl *M) {
- return M->getOverloadedOperator() == OO_Star;
- });
+ DeclarationName OpStar =
+ RD->getASTContext().DeclarationNames.getCXXOperatorName(OO_Star);
+ return !RD->lookup(OpStar).empty();
}
static StringRef findValueMethod(const CXXRecordDecl *RD) {
- for (const auto *M : RD->methods()) {
- if (!M->getDeclName().isIdentifier())
- continue;
- StringRef Name = M->getName();
- if (Name == "value" || Name == "Value")
+ ASTContext &Ctx = RD->getASTContext();
+ for (StringRef Name : {"value", "Value"}) {
+ DeclarationName DN = &Ctx.Idents.get(Name);
+ if (!RD->lookup(DN).empty())
return Name;
}
return {};
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
index c06467676ff4c..1cc7367a07124 100644
--- 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
@@ -1,12 +1,12 @@
// RUN: %check_clang_tidy -check-suffixes=NON-OWNING %s \
// RUN: performance-expensive-value-or %t -- \
// RUN: -config='{CheckOptions: { \
-// RUN: performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional" \
+// RUN: performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional;::inherited::Optional" \
// RUN: }}'
// RUN: %check_clang_tidy -check-suffixes=OWNING %s \
// RUN: performance-expensive-value-or %t -- \
// RUN: -config='{CheckOptions: { \
-// RUN: performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional", \
+// RUN: performance-expensive-value-or.OptionalTypes: "::std::optional;::absl::optional;::custom::CamelOptional;::custom::PascalOptional;::inherited::Optional", \
// RUN: performance-expensive-value-or.WarnOnOwnershipTaking: true \
// RUN: }}'
@@ -197,3 +197,35 @@ void positiveMacro(std::optional<std::string> opt) {
auto val = GET_OR_DEFAULT(opt, "default");
// CHECK-MESSAGES-OWNING: :[[@LINE-1]]:14: warning: 'value_or' copies expensive type 'std::basic_string<char>'
}
+
+// Inherited methods via using declarations (libc++ pattern).
+
+namespace inherited {
+
+template <typename T>
+struct OptionalBase {
+ const T &operator*() const & noexcept;
+ T &operator*() & noexcept;
+ T &value() &;
+ const T &value() const &;
+};
+
+template <typename T>
+struct Optional : OptionalBase<T> {
+ using OptionalBase<T>::operator*;
+ using OptionalBase<T>::value;
+
+ template <class U = T>
+ T value_or(U &&default_value) const &;
+};
+
+} // namespace inherited
+
+void positiveInheritedRef(inherited::Optional<std::string> opt,
+ const std::string &fallback) {
+ const std::string &ref = opt.value_or(fallback);
+ // CHECK-MESSAGES-NON-OWNING: :[[@LINE-1]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback
+ // CHECK-MESSAGES-OWNING: :[[@LINE-2]]:32: warning: 'value_or' copies expensive type 'std::basic_string<char>'; consider using 'operator*' or 'value()' with a separate fallback
+ // CHECK-FIXES-NON-OWNING: const std::string &ref = (opt ? *opt : fallback);
+ // CHECK-FIXES-OWNING: const std::string &ref = (opt ? *opt : fallback);
+}
>From b962c57311a61c0e181963810e470d1a9eccbf87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Fri, 10 Jul 2026 15:50:44 +0200
Subject: [PATCH 26/26] fix const-correctness lint warnings
---
.../clang-tidy/performance/ExpensiveValueOrCheck.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
index a744c6656a76b..2c702fc5cbb8e 100644
--- a/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ExpensiveValueOrCheck.cpp
@@ -19,7 +19,7 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
static bool hasOperatorStar(const CXXRecordDecl *RD) {
- DeclarationName OpStar =
+ const DeclarationName OpStar =
RD->getASTContext().DeclarationNames.getCXXOperatorName(OO_Star);
return !RD->lookup(OpStar).empty();
}
@@ -27,7 +27,7 @@ static bool hasOperatorStar(const CXXRecordDecl *RD) {
static StringRef findValueMethod(const CXXRecordDecl *RD) {
ASTContext &Ctx = RD->getASTContext();
for (StringRef Name : {"value", "Value"}) {
- DeclarationName DN = &Ctx.Idents.get(Name);
+ const DeclarationName DN = &Ctx.Idents.get(Name);
if (!RD->lookup(DN).empty())
return Name;
}
More information about the cfe-commits
mailing list