[clang-tidy] Add bugprone-optional-value-conversion check

[PiotrZSL]

Summary

Detects potentially unintentional and redundant conversions where a value is
extracted from an optional-like type and then used to create a new instance of
the same optional-like type.

[Eugene.Zelenko]

clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst

47

Unintended quote in value.

[PiotrZSL]

There are 37 findings in llvm repository:

1. clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:96:12: warning: conversion from 'std::optional<bool>' into 'bool' and back into 'std::optional<bool>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
2. clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:65:12: warning: conversion from 'std::optional<std::basic_string<char>>' into 'std::basic_string<char>' and back into 'std::optional<std::basic_string<char>>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
3. clang/lib/AST/Interp/Program.cpp:173:12: warning: conversion from 'std::optional<unsigned int>' into 'unsigned int' and back into 'std::optional<unsigned int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
4. clang/lib/Analysis/PathDiagnostic.cpp:323:14: warning: conversion from 'std::optional<bool>' into 'bool' and back into 'std::optional<bool>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
5. clang/lib/Frontend/Rewrite/InclusionRewriter.cpp:253:7: warning: conversion from 'std::optional<llvm::MemoryBufferRef>' into 'llvm::MemoryBufferRef' and back into 'std::optional<llvm::MemoryBufferRef>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
6. clang/lib/Sema/Scope.cpp:190:25: warning: conversion from 'std::optional<clang::VarDecl *>' into 'clang::VarDecl *' and back into 'std::optional<clang::VarDecl *>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
7. clang/lib/Sema/TreeTransform.h:13915:9: warning: conversion from 'std::optional<unsigned int>' into 'unsigned int' and back into 'std::optional<unsigned int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
8. clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:440:16: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
9. clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp:125:10: warning: conversion from 'std::optional<bool>' into 'bool' and back into 'std::optional<bool>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
10. clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1573:10: warning: conversion from 'std::optional<clang::ento::nonloc::LazyCompoundVal>' into 'clang::ento::nonloc::LazyCompoundVal' and back into 'std::optional<clang::ento::nonloc::LazyCompoundVal>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
11. clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1784:12: warning: conversion from 'std::optional<clang::ento::SVal>' into 'clang::ento::SVal' and back into 'std::optional<clang::ento::SVal>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
12. clang/lib/Tooling/InterpolatingCompilationDatabase.cpp:189:16: warning: conversion from 'std::optional<clang::driver::types::ID>' into 'clang::driver::types::ID' and back into 'std::optional<clang::driver::types::ID>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
13. lldb/source/Breakpoint/BreakpointResolver.cpp:233:33: warning: conversion from 'std::optional<unsigned short>' into 'unsigned short' and back into 'std::optional<unsigned short>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
14. lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1362:53: warning: conversion from 'std::optional<unsigned long>' into 'unsigned long' and back into 'std::optional<unsigned long>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
15. llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:4119:12: warning: conversion from 'std::optional<LiveDebugValues::ValueIDNum>' into 'LiveDebugValues::ValueIDNum' and back into 'std::optional<LiveDebugValues::ValueIDNum>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
16. llvm/lib/DWARFLinker/DWARFLinker.cpp:1191:14: warning: conversion from 'std::optional<unsigned long>' into 'unsigned long' and back into 'std::optional<unsigned long>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
17. llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1305:31: warning: conversion from 'std::optional<long>' into 'long' and back into 'std::optional<long>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
18. llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1322:62: warning: conversion from 'std::optional<llvm::DWARFFormValue>' into 'llvm::DWARFFormValue' and back into 'std::optional<llvm::DWARFFormValue>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
19. llvm/lib/DebugInfo/Symbolize/Markup.cpp:67:14: warning: conversion from 'std::optional<llvm::symbolize::MarkupNode>' into 'llvm::symbolize::MarkupNode' and back into 'std::optional<llvm::symbolize::MarkupNode>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
20. llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp:425:13: warning: conversion from 'std::optional<llvm::raw_ostream::Colors>' into 'llvm::raw_ostream::Colors' and back into 'std::optional<llvm::raw_ostream::Colors>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
21. llvm/lib/InterfaceStub/IFSHandler.cpp:231:24: warning: conversion from 'std::optional<unsigned short>' into 'unsigned short' and back into 'std::optional<unsigned short>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
22. llvm/lib/InterfaceStub/IFSHandler.cpp:239:30: warning: conversion from 'std::optional<llvm::ifs::IFSEndiannessType>' into 'llvm::ifs::IFSEndiannessType' and back into 'std::optional<llvm::ifs::IFSEndiannessType>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
23. llvm/lib/InterfaceStub/IFSHandler.cpp:246:28: warning: conversion from 'std::optional<llvm::ifs::IFSBitWidthType>' into 'llvm::ifs::IFSBitWidthType' and back into 'std::optional<llvm::ifs::IFSBitWidthType>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
24. llvm/lib/LTO/LTOBackend.cpp:214:18: warning: conversion from 'std::optional<llvm::Reloc::Model>' into 'llvm::Reloc::Model' and back into 'std::optional<llvm::Reloc::Model>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
25. llvm/lib/LTO/LTOBackend.cpp:221:17: warning: conversion from 'std::optional<llvm::CodeModel::Model>' into 'llvm::CodeModel::Model' and back into 'std::optional<llvm::CodeModel::Model>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
26. llvm/lib/Remarks/BitstreamRemarkParser.cpp:563:17: warning: conversion from 'std::optional<unsigned long>' into 'unsigned long' and back into 'std::optional<unsigned long>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
27. llvm/lib/Support/UnicodeNameToCodepoint.cpp:382:12: warning: conversion from 'std::optional<char32_t>' into 'char32_t' and back into 'std::optional<char32_t>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
28. llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1028:12: warning: conversion from 'std::optional<llvm::Value *>' into 'llvm::Value *' and back into 'std::optional<llvm::Value *>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
29. llvm/lib/Transforms/Utils/SimplifyCFG.cpp:4651:33: warning: conversion from 'std::optional<unsigned int>' into 'unsigned int' and back into 'std::optional<unsigned int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
30. llvm/lib/Transforms/Utils/ValueMapper.cpp:576:12: warning: conversion from 'std::optional<llvm::Metadata *>' into 'llvm::Metadata *' and back into 'std::optional<llvm::Metadata *>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
31. llvm/lib/Transforms/Utils/ValueMapper.cpp:615:12: warning: conversion from 'std::optional<llvm::Metadata *>' into 'llvm::Metadata *' and back into 'std::optional<llvm::Metadata *>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
32. llvm/lib/Transforms/Utils/ValueMapper.cpp:832:12: warning: conversion from 'std::optional<llvm::Metadata *>' into 'llvm::Metadata *' and back into 'std::optional<llvm::Metadata *>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
33. llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:435:14: warning: conversion from 'std::optional<unsigned int>' into 'unsigned int' and back into 'std::optional<unsigned int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
34. llvm/tools/llvm-lto2/llvm-lto2.cpp:264:23: warning: conversion from 'std::optional<llvm::Reloc::Model>' into 'llvm::Reloc::Model' and back into 'std::optional<llvm::Reloc::Model>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
35. llvm/tools/llvm-profgen/ProfiledBinary.cpp:104:14: warning: conversion from 'std::optional<unsigned int>' into 'unsigned int' and back into 'std::optional<unsigned int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
36. llvm/tools/llvm-profgen/ProfiledBinary.cpp:118:16: warning: conversion from 'std::optional<unsigned int>' into 'unsigned int' and back into 'std::optional<unsigned int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
37. llvm/utils/TableGen/GlobalISelEmitter.cpp:5217:12: warning: conversion from 'std::optional<const llvm::CodeGenRegisterClass *>' into 'const llvm::CodeGenRegisterClass *' and back into 'std::optional<const llvm::CodeGenRegisterClass *>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]

[PiotrZSL]

Fix doc

[njames93]

Have you thought of the possibility of handling this case

takeOptionalValue(std::move(*param));

clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst

35

Any reason for this limitation, given in most cases the fix is to just remove the * or .value() call

clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

17

It'd be good to have tests demonstating boost::optional and absl::optional as well as the (non standard) get accessor

[PiotrZSL]

As for takeOptionalValue(std::move(*param));, it could be added as an configuration option for PassthroughFunctions or UtilityFunctions. I can thing about that, that should be easy to implement.

clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst

35

Sometimes problem is not with optional but with called function that take optional when it could take value.
At least for such issues I run, still probably could be safe to introduce such auto-fix to just remove */.value().
But then question would be if it should be bugprone or readability check.
For me like 10% of these issues show some possible bug around that code.

clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

17

I added them to config just to add them, as they should work, I could add some tests for them, but behavior should be same...

[njames93]

In D147357#4256636, @PiotrZSL wrote:

As for takeOptionalValue(std::move(*param));, it could be added as an configuration option for PassthroughFunctions or UtilityFunctions. I can thing about that, that should be easy to implement.

That's overcomplicating things, just look for std::move and you'll cover 99.9% of instances in real world code.

clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst

35

That's a good point, if they are hiding a bug we don't want to auto fix it.
Perhaps the fix should be added as a note

note: Remove the call to '%FuncName%' to silence this warning
note: Remove '*' to silence this warning

clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

17

On the subject of boost(unsure about absl as I've never used that library)
Would you ever add support for T& boost::get(boost::optional<T> &) or its const ref cousin?

38

What happens with this pathological case
takeOptionalValue(param.operator*());
Looks like it isn't being handled, should probably handle(and test) for it

[PiotrZSL]

Add fixes, add suport for std:move, fixed operator*() calls, added more tests

[PiotrZSL]

TODO: Fix pointers, Add more tests, consider free standing value extraction functions like boost::get

[PiotrZSL]

clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

17

No, I'm not planing to add support for boost::get. I added basic support for boost and absl only because it's cheap, and interfaces are similar. Main target is std::optional. And configuration is provider just because for example in project that I work for we got custom optional implementation.
Added info about limited support for non-standard optional.

[PiotrZSL]

Added support for ->, added more tests.

[PiotrZSL]

Ready for review.

[PiotrZSL]

Marked check as one that provides fixes.

[PiotrZSL]

Ping

[PiotrZSL]

Ping

[PiotrZSL]

Ping

[PiotrZSL]

Rebase + Ping

[xgupta]

clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp

62

Any good reason to choose "fun" as the matcher name?

clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst

33

Can we also write - A better approach would be to directly pass opt to the print function without extracting its value:
print(opt)

[PiotrZSL]

clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp

62

shortcut from function, as its matcher for a cxxMemberCallExpr i will change it to for example "call"

clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst

33

Yes I will add such thing...

[PiotrZSL]

Review comments

[xgupta]

LGTM, Thanks!

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultiLevelImplicitPointerConversionCheck.h"
 #include "MultipleNewInOneExpressionCheck.h"
 #include "MultipleStatementMacroCheck.h"
 #include "NoEscapeCheck.h"
 #include "NonZeroEnumToBoolConversionCheck.h"
 #include "NotNullTerminatedResultCheck.h"
+#include "OptionalValueConversionCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
 #include "SharedPtrArrayMismatchCheck.h"
 #include "SignalHandlerCheck.h"
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<MoveForwardingReferenceCheck>(
         "bugprone-move-forwarding-reference");
     CheckFactories.registerCheck<MultiLevelImplicitPointerConversionCheck>(
         "bugprone-multi-level-implicit-pointer-conversion");
     CheckFactories.registerCheck<MultipleNewInOneExpressionCheck>(
         "bugprone-multiple-new-in-one-expression");
     CheckFactories.registerCheck<MultipleStatementMacroCheck>(
         "bugprone-multiple-statement-macro");
+    CheckFactories.registerCheck<OptionalValueConversionCheck>(
+        "bugprone-optional-value-conversion");
     CheckFactories.registerCheck<RedundantBranchConditionCheck>(
         "bugprone-redundant-branch-condition");
     CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
         "bugprone-narrowing-conversions");
     CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");
     CheckFactories.registerCheck<NonZeroEnumToBoolConversionCheck>(
         "bugprone-non-zero-enum-to-bool-conversion");
     CheckFactories.registerCheck<NotNullTerminatedResultCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultiLevelImplicitPointerConversionCheck.cpp
   MultipleNewInOneExpressionCheck.cpp
   MultipleStatementMacroCheck.cpp
   NoEscapeCheck.cpp
   NonZeroEnumToBoolConversionCheck.cpp
   NotNullTerminatedResultCheck.cpp
+  OptionalValueConversionCheck.cpp
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp
   SharedPtrArrayMismatchCheck.cpp
   SignalHandlerCheck.cpp
   SignedCharMisuseCheck.cpp
   SizeofContainerCheck.cpp

clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h

+//===--- OptionalValueConversionCheck.h - clang-tidy ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include <vector>
+
+namespace clang::tidy::bugprone {
+
+/// Detects potentially unintentional and redundant conversions where a value is
+/// extracted from an optional-like type and then used to create a new instance
+/// of the same optional-like type.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/optional-value-conversion.html
+class OptionalValueConversionCheck : public ClangTidyCheck {
+public:
+  OptionalValueConversionCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override;
+
+private:
+  std::vector<StringRef> OptionalTypes;
+  std::vector<StringRef> ValueMethods;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H

clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp

+//===--- OptionalValueConversionCheck.cpp - clang-tidy --------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "OptionalValueConversionCheck.h"
+#include "../utils/LexerUtils.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(QualType, hasCleanType, ast_matchers::internal::Matcher<QualType>,
+              InnerMatcher) {
+  return InnerMatcher.matches(
+      Node.getNonReferenceType().getUnqualifiedType().getCanonicalType(),
+      Finder, Builder);
+}
+
+} // namespace
+
+OptionalValueConversionCheck::OptionalValueConversionCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      OptionalTypes(utils::options::parseStringList(
+          Options.get("OptionalTypes",
+                      "::std::optional;::absl::optional;::boost::optional"))),
+      ValueMethods(utils::options::parseStringList(
+          Options.get("ValueMethods", "::value$;::get$"))) {}
+
+std::optional<TraversalKind>
+OptionalValueConversionCheck::getCheckTraversalKind() const {
+  return TK_AsIs;
+}
+
+void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
+  auto ConstructTypeMatcher =
+      qualType(hasCleanType(qualType().bind("optional-type")));
+
+  auto CallTypeMatcher =
+      qualType(hasCleanType(equalsBoundNode("optional-type")));
+
+  auto OptionalDereferenceMatcher = callExpr(
+      anyOf(
+          cxxOperatorCallExpr(hasOverloadedOperatorName("*"),
+                              hasUnaryOperand(hasType(CallTypeMatcher)))
+              .bind("op-call"),
+          cxxMemberCallExpr(thisPointerType(CallTypeMatcher),
+                            callee(cxxMethodDecl(anyOf(
+                                hasOverloadedOperatorName("*"),
+                                matchers::matchesAnyListedName(ValueMethods)))))
+              .bind("member-call")),
+      hasType(qualType().bind("value-type")));
+
+  auto StdMoveCallMatcher =
+      callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))),
+               hasArgument(0, ignoringImpCasts(OptionalDereferenceMatcher)));
+  Finder->addMatcher(
+      cxxConstructExpr(
+          argumentCountIs(1U),
+          hasDeclaration(cxxConstructorDecl(
+              ofClass(matchers::matchesAnyListedName(OptionalTypes)))),
+          hasType(ConstructTypeMatcher),
+          hasArgument(0U, ignoringImpCasts(anyOf(OptionalDereferenceMatcher,
+                                                 StdMoveCallMatcher))))
+          .bind("expr"),
+      this);
+}
+
+void OptionalValueConversionCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "OptionalTypes",
+                utils::options::serializeStringList(OptionalTypes));
+  Options.store(Opts, "ValueMethods",
+                utils::options::serializeStringList(ValueMethods));
+}
+
+void OptionalValueConversionCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedExpr = Result.Nodes.getNodeAs<Expr>("expr");
+  const auto *OptionalType = Result.Nodes.getNodeAs<QualType>("optional-type");
+  const auto *ValueType = Result.Nodes.getNodeAs<QualType>("value-type");
+
+  diag(MatchedExpr->getExprLoc(),
+       "conversion from %0 into %1 and back into %0, remove potentially "
+       "error-prone optional dereference")
+      << *OptionalType << ValueType->getUnqualifiedType();
+
+  if (const auto *OperatorExpr =
+          Result.Nodes.getNodeAs<CXXOperatorCallExpr>("op-call")) {
+    diag(OperatorExpr->getExprLoc(), "remove '*' to silence this warning",
+         DiagnosticIDs::Note)
+        << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+               OperatorExpr->getBeginLoc(), OperatorExpr->getExprLoc()));
+    return;
+  }
+  if (const auto *CallExpr =
+          Result.Nodes.getNodeAs<CXXMemberCallExpr>("member-call")) {
+    const SourceLocation Begin =
+        utils::lexer::getPreviousToken(CallExpr->getExprLoc(),
+                                       *Result.SourceManager, getLangOpts())
+            .getLocation();
+    auto Diag =
+        diag(CallExpr->getExprLoc(),
+             "remove call to %0 to silence this warning", DiagnosticIDs::Note);
+    Diag << CallExpr->getMethodDecl()
+         << FixItHint::CreateRemoval(
+                CharSourceRange::getTokenRange(Begin, CallExpr->getEndLoc()));
+    if (const auto *Member =
+            llvm::dyn_cast<MemberExpr>(CallExpr->getCallee()->IgnoreImplicit());
+        Member && Member->isArrow())
+      Diag << FixItHint::CreateInsertion(CallExpr->getBeginLoc(), "*");
+    return;
+  }
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-inc-dec-in-conditions
   the variable's value.
 
 - New :doc:`bugprone-multi-level-implicit-pointer-conversion
   <clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion>` check.
 
   Detects implicit conversions between pointers of different levels of
   indirection.
 
+- New :doc:`bugprone-optional-value-conversion
+  <clang-tidy/checks/bugprone/optional-value-conversion>` check.
+
+  Detects potentially unintentional and redundant conversions where a value is
+  extracted from an optional-like type and then used to create a new instance
+  of the same optional-like type.
+
 - New :doc:`modernize-use-constraints
   <clang-tidy/checks/modernize/use-constraints>` check.
 
   Replace ``enable_if`` with C++20 requires clauses.
 
 - New :doc:`performance-enum-size
   <clang-tidy/checks/performance/enum-size>` check.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst

+.. title:: clang-tidy - bugprone-optional-value-conversion
+
+bugprone-optional-value-conversion
+==================================
+
+Detects potentially unintentional and redundant conversions where a value is
+extracted from an optional-like type and then used to create a new instance of
+the same optional-like type.
+
+These conversions might be the result of developer oversight, leftovers from
+code refactoring, or other situations that could lead to unintended exceptions
+or cases where the resulting optional is always initialized, which might be
+unexpected behavior.
+
+To illustrate, consider the following problematic code snippet:
+
+.. code-block:: c++
+
+    #include <optional>
+
+    void print(std::optional<int>);
+
+    int main()
+    {
+      std::optional<int> opt;
+      // ...
+
+      // Unintentional conversion from std::optional<int> to int and back to
+      // std::optional<int>:
+      print(opt.value());
+
+      // ...
+    }
+
+A better approach would be to directly pass ``opt`` to the ``print`` function
+without extracting its value:
+
+.. code-block:: c++
+
+    #include <optional>
+
+    void print(std::optional<int>);
+
+    int main()
+    {
+      std::optional<int> opt;
+      // ...
+
+      // Proposed code: Directly pass the std::optional<int> to the print
+      // function.
+      print(opt);
+
+      // ...
+    }
+
+By passing ``opt`` directly to the print function, unnecessary conversions are
+avoided, and potential unintended behavior or exceptions are minimized.
+
+Value extraction using ``operator *`` is matched by default.
+The support for non-standard optional types such as ``boost::optional`` or
+``absl::optional`` may be limited.
+
+Options:
+--------
+
+.. option:: OptionalTypes
+
+    Semicolon-separated list of (fully qualified) optional type names or regular
+    expressions that match the optional types.
+    Default value is `::std::optional;::absl::optional;::boost::optional`.
+
+.. option:: ValueMethods
+
+    Semicolon-separated list of (fully qualified) method names or regular
+    expressions that match the methods.
+    Default value is `::value$;::get$`.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `bugprone-misplaced-widening-cast <bugprone/misplaced-widening-cast.html>`_,
    `bugprone-move-forwarding-reference <bugprone/move-forwarding-reference.html>`_, "Yes"
    `bugprone-multi-level-implicit-pointer-conversion <bugprone/multi-level-implicit-pointer-conversion.html>`_,
    `bugprone-multiple-new-in-one-expression <bugprone/multiple-new-in-one-expression.html>`_,
    `bugprone-multiple-statement-macro <bugprone/multiple-statement-macro.html>`_,
    `bugprone-no-escape <bugprone/no-escape.html>`_,
    `bugprone-non-zero-enum-to-bool-conversion <bugprone/non-zero-enum-to-bool-conversion.html>`_,
    `bugprone-not-null-terminated-result <bugprone/not-null-terminated-result.html>`_, "Yes"
+   `bugprone-optional-value-conversion <bugprone/optional-value-conversion.html>`_, "Yes"
    `bugprone-parent-virtual-call <bugprone/parent-virtual-call.html>`_, "Yes"
    `bugprone-posix-return <bugprone/posix-return.html>`_, "Yes"
    `bugprone-redundant-branch-condition <bugprone/redundant-branch-condition.html>`_, "Yes"
    `bugprone-reserved-identifier <bugprone/reserved-identifier.html>`_, "Yes"
    `bugprone-shared-ptr-array-mismatch <bugprone/shared-ptr-array-mismatch.html>`_, "Yes"
    `bugprone-signal-handler <bugprone/signal-handler.html>`_,
    `bugprone-signed-char-misuse <bugprone/signed-char-misuse.html>`_,
    `bugprone-sizeof-container <bugprone/sizeof-container.html>`_,

clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t -- --fix-notes
+// RUN: %check_clang_tidy -check-suffix=CUSTOM -std=c++17-or-later %s bugprone-optional-value-conversion %t -- \
+// RUN: -config="{CheckOptions: [{key: 'bugprone-optional-value-conversion.OptionalTypes', value: 'CustomOptional'}, \
+// RUN:                          {key: 'bugprone-optional-value-conversion.ValueMethods', value: '::Read$;::Ooo$'}]}" --fix-notes
+
+namespace std {
+  template<typename T>
+  struct optional
+  {
+    constexpr optional() noexcept;
+    constexpr optional(T&&) noexcept;
+    constexpr optional(const T&) noexcept;
+    template<typename U>
+    constexpr optional(U&&) noexcept;
+    const T& operator*() const;
+    T* operator->();
+    const T* operator->() const;
+    T& operator*();
+    const T& value() const;
+    T& value();
+    const T& get() const;
+    T& get();
+    T value_or(T) const;
+  };
+
+  template <class T>
+  T&& move(T &x) {
+    return static_cast<T&&>(x);
+  }
+}
+
+namespace boost {
+  template<typename T>
+  struct optional {
+    constexpr optional() noexcept;
+    constexpr optional(const T&) noexcept;
+    const T& operator*() const;
+    const T& get() const;
+  };
+}
+
+namespace absl {
+  template<typename T>
+  struct optional {
+    constexpr optional() noexcept;
+    constexpr optional(const T&) noexcept;
+    const T& operator*() const;
+    const T& value() const;
+  };
+}
+
+template<typename T>
+struct CustomOptional {
+  CustomOptional();
+  CustomOptional(const T&);
+  const T& Read() const;
+  T& operator*();
+  T& Ooo();
+};
+
+void takeOptionalValue(std::optional<int>);
+void takeOptionalRef(const std::optional<int>&);
+void takeOptionalRRef(std::optional<int>&&);
+void takeOtherOptional(std::optional<long>);
+void takeBOptionalValue(boost::optional<int>);
+void takeAOptionalValue(absl::optional<int>);
+
+void incorrect(std::optional<int> param)
+{
+  std::optional<int>* ptr = &param;
+  takeOptionalValue(**ptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(ptr->value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(param.operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(ptr->operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalRef(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(ptr->value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(*ptr);
+  takeOptionalRef(param.operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(ptr->operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(*ptr);
+  std::optional<int> p = *param;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: std::optional<int> p = param;
+
+  takeOptionalValue(std::move(**ptr));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(*ptr));
+  takeOptionalValue(std::move(*param));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(param));
+  takeOptionalValue(std::move(param.value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(param));
+  takeOptionalValue(std::move(ptr->value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(*ptr));
+  takeOptionalValue(std::move(param.operator*()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(param));
+  takeOptionalRef(std::move(*param));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(std::move(param));
+  takeOptionalRef(std::move(param.value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(std::move(param));
+  takeOptionalRef(std::move(ptr->value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(std::move(*ptr));
+  takeOptionalRef(std::move(param.operator*()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(std::move(param));
+  takeOptionalRRef(std::move(*param));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(param));
+  takeOptionalRRef(std::move(param.value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(param));
+  takeOptionalRRef(std::move(ptr->value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(*ptr));
+  takeOptionalRRef(std::move(param.operator*()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(param));
+  takeOptionalRRef(std::move(ptr->operator*()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(*ptr));
+  std::optional<int> p2 = std::move(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: std::optional<int> p2 = std::move(param);
+
+  std::optional<std::optional<int>> opt;
+  takeOptionalValue(opt->value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*opt);
+  takeOptionalValue(opt->operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*opt);
+
+  boost::optional<int> bopt;
+  takeBOptionalValue(*bopt);
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: conversion from 'boost::optional<int>' into 'int' and back into 'boost::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeBOptionalValue(bopt);
+  takeBOptionalValue(bopt.get());
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: conversion from 'boost::optional<int>' into 'int' and back into 'boost::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeBOptionalValue(bopt);
+
+  absl::optional<int> aopt;
+  takeAOptionalValue(*aopt);
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: conversion from 'absl::optional<int>' into 'int' and back into 'absl::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeAOptionalValue(aopt);
+  takeAOptionalValue(aopt.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: conversion from 'absl::optional<int>' into 'int' and back into 'absl::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeAOptionalValue(aopt);
+}
+
+void takeCustom(const CustomOptional<int>&);
+
+void testCustom(CustomOptional<int> param) {
+  takeCustom(*param);
+  // CHECK-MESSAGES-CUSTOM: :[[@LINE-1]]:14: warning: conversion from 'CustomOptional<int>' into 'int' and back into 'CustomOptional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES-CUSTOM: takeCustom(param);
+  takeCustom(param.Read());
+  // CHECK-MESSAGES-CUSTOM: :[[@LINE-1]]:14: warning: conversion from 'CustomOptional<int>' into 'int' and back into 'CustomOptional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES-CUSTOM: takeCustom(param);
+  takeCustom(param.Ooo());
+  // CHECK-MESSAGES-CUSTOM: :[[@LINE-1]]:14: warning: conversion from 'CustomOptional<int>' into 'int' and back into 'CustomOptional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES-CUSTOM: takeCustom(param);
+}
+
+void correct(std::optional<int> param)
+{
+  takeOtherOptional(*param);
+  takeOtherOptional(param.value());
+  takeOtherOptional(param.value_or(5U));
+  takeOtherOptional(param.operator*());
+
+  std::optional<long> p = *param;
+  takeOptionalValue(param.value_or(5U));
+  takeOptionalRef(param.value_or(5U));
+
+  std::optional<int>* ptr = &param;
+  takeOtherOptional(**ptr);
+  takeOtherOptional(ptr->value());
+  takeOtherOptional(ptr->value_or(5U));
+  takeOtherOptional(ptr->operator*());
+
+  std::optional<long>* p2 = &p;
+  takeOptionalValue(p2->value_or(5U));
+  takeOptionalRef(p2->value_or(5U));
+}