[clang-tidy] Allow bugprone-unchecked-optional-access to handle calls to `std::forward`

[AMS21]

Summary

The check now understands that calling std::forward
will not modify the underlying optional value.

This fixes llvm#59705

[PiotrZSL]

LGTM

clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

589–591

if (LocArg == nullptr)
    return;

[AMS21]

Remove unneccesary braces

[AMS21]

Now I would kindly as for someone to commit this on my behalf.

clang-tools-extra/docs/ReleaseNotes.rst

- Deprecated check-local options `HeaderFileExtensions`
   <clang-tidy/checks/bugprone/dynamic-static-initializers>` check.
   Global options of the same name should be used instead.
 
 - Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
   in :doc:`bugprone-suspicious-include
   <clang-tidy/checks/bugprone/suspicious-include>` check.
   Global options of the same name should be used instead.
 
+- Improved :doc:`bugprone-unchecked-optional-access
+  <clang-tidy/decks/bugprone/unchecked-optional-access>` check to properly handle calls
+  to ``std::forward``.
+
 - Improved :doc:`bugprone-use-after-move
   <clang-tidy/checks/bugprone/use-after-move>` check to also cover constructor
   initializers.
 
 - Deprecated check-local options `HeaderFileExtensions`
   in :doc:`google-build-namespaces
   <clang-tidy/checks/google/build-namespaces>` check.
   Global options of the same name should be used instead.

clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp

 }
 
 class C4 {
   explicit C4(absl::optional<int> opt) : foo_(opt.value()) {
     // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: unchecked access to optional
   }
   int foo_;
 };
+
+// llvm#59705
+namespace std
+{
+  template <typename T>
+  constexpr T&& forward(T& type) noexcept {
+    return static_cast<T&&>(type);
+  }
+
+  template <typename T>
+  constexpr T&& forward(T&& type) noexcept {
+    return static_cast<T&&>(type);
+  }
+}
+
+void std_forward_copy(absl::optional<int> opt) {
+  std::forward<absl::optional<int>>(opt).value();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional
+}
+
+void std_forward_copy_safe(absl::optional<int> opt) {
+  if (!opt) return;
+
+  std::forward<absl::optional<int>>(opt).value();
+}
+
+void std_forward_copy(absl::optional<int>& opt) {
+  std::forward<absl::optional<int>>(opt).value();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional
+}
+
+void std_forward_lvalue_ref_safe(absl::optional<int>& opt) {
+  if (!opt) return;
+
+  std::forward<absl::optional<int>>(opt).value();
+}
+
+void std_forward_copy(absl::optional<int>&& opt) {
+  std::forward<absl::optional<int>>(opt).value();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional
+}
+
+void std_forward_rvalue_ref_safe(absl::optional<int>&& opt) {
+  if (!opt) return;
+
+  std::forward<absl::optional<int>>(opt).value();
+}

clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

 auto hasNulloptType() { return hasType(nulloptTypeDecl()); }
 
 // `optional` or `nullopt_t`
 auto hasAnyOptionalType() {
   return hasType(hasUnqualifiedDesugaredType(
       recordType(hasDeclaration(anyOf(nulloptTypeDecl(), optionalClass())))));
 }
 
-
 auto inPlaceClass() {
   return recordDecl(
       hasAnyName("std::in_place_t", "absl::in_place_t", "base::in_place_t"));
 }
 
 auto isOptionalNulloptConstructor() {
   return cxxConstructExpr(
       hasOptionalType(),

 }
 
 auto isStdSwapCall() {
   return callExpr(callee(functionDecl(hasName("std::swap"))),
                   argumentCountIs(2), hasArgument(0, hasOptionalType()),
                   hasArgument(1, hasOptionalType()));
 }
 
+auto isStdForwardCall() {
+  return callExpr(callee(functionDecl(hasName("std::forward"))),
+                  argumentCountIs(1), hasArgument(0, hasOptionalType()));
+}
+
 constexpr llvm::StringLiteral ValueOrCallID = "ValueOrCall";
 
 auto isValueOrStringEmptyCall() {
   // `opt.value_or("").empty()`
   return cxxMemberCallExpr(
       callee(cxxMethodDecl(hasName("empty"))),
       onImplicitObjectArgument(ignoringImplicit(
           cxxMemberCallExpr(on(expr(unless(cxxThisExpr()))),

 }
 
 void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &,
                          LatticeTransferState &State) {
   assert(E->getNumArgs() == 2);
   transferSwap(*E->getArg(0), SkipPast::Reference, *E->getArg(1), State.Env);
 }
 
+void transferStdForwardCall(const CallExpr *E, const MatchFinder::MatchResult &,
+                            LatticeTransferState &State) {
+  assert(E->getNumArgs() == 1);
+
+  StorageLocation *LocRet = State.Env.getStorageLocation(*E, SkipPast::None);
+  if (LocRet != nullptr)
+    return;
+
+  StorageLocation *LocArg =
+      State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference);
+
+  if (LocArg == nullptr) {
+    return;
+  }
+
+  Value *ValArg = State.Env.getValue(*LocArg);
+  if (ValArg == nullptr)
+    ValArg = &createOptionalValue(State.Env, State.Env.makeAtomicBoolValue());
+
+  // Create a new storage location
+  LocRet = &State.Env.createStorageLocation(*E);
+  State.Env.setStorageLocation(*E, *LocRet);
+
+  State.Env.setValue(*LocRet, *ValArg);
+}
+
 BoolValue &evaluateEquality(Environment &Env, BoolValue &EqVal, BoolValue &LHS,
                             BoolValue &RHS) {
   // Logically, an optional<T> object is composed of two values - a `has_value`
   // bit and a value of type T. Equality of optional objects compares both
   // values. Therefore, merely comparing the `has_value` bits isn't sufficient:
   // when two optional objects are engaged, the equality of their respective
   // values of type T matters. Since we only track the `has_value` bits, we
   // can't make any conclusions about equality when we know that two optional
return CFGMatchSwitchBuilder<LatticeTransferState>()
              LatticeTransferState &State) {
             assignOptionalValue(*E, State.Env,
                                 State.Env.getBoolLiteralValue(false));
           })
       // optional::optional (value/conversion)
       .CaseOfCFGStmt<CXXConstructExpr>(isOptionalValueOrConversionConstructor(),
                                        transferValueOrConversionConstructor)
 
-
       // optional::operator=
       .CaseOfCFGStmt<CXXOperatorCallExpr>(
           isOptionalValueOrConversionAssignment(),
           transferValueOrConversionAssignment)
       .CaseOfCFGStmt<CXXOperatorCallExpr>(isOptionalNulloptAssignment(),
                                           transferNulloptAssignment)
 
       // optional::value
return CFGMatchSwitchBuilder<LatticeTransferState>()
 
       // optional::swap
       .CaseOfCFGStmt<CXXMemberCallExpr>(isOptionalMemberCallWithName("swap"),
                                         transferSwapCall)
 
       // std::swap
       .CaseOfCFGStmt<CallExpr>(isStdSwapCall(), transferStdSwapCall)
 
+      // std::forward
+      .CaseOfCFGStmt<CallExpr>(isStdForwardCall(), transferStdForwardCall)
+
       // opt.value_or("").empty()
       .CaseOfCFGStmt<Expr>(isValueOrStringEmptyCall(),
                            transferValueOrStringEmptyCall)
 
       // opt.value_or(X) != X
       .CaseOfCFGStmt<Expr>(isValueOrNotEqX(), transferValueOrNotEqX)
 
       // Comparisons (==, !=):

 
 ComparisonResult UncheckedOptionalAccessModel::compare(
     QualType Type, const Value &Val1, const Environment &Env1,
     const Value &Val2, const Environment &Env2) {
   if (!isOptionalType(Type))
     return ComparisonResult::Unknown;
   bool MustNonEmpty1 = isNonEmptyOptional(Val1, Env1);
   bool MustNonEmpty2 = isNonEmptyOptional(Val2, Env2);
-  if (MustNonEmpty1 && MustNonEmpty2) return ComparisonResult::Same;
+  if (MustNonEmpty1 && MustNonEmpty2)
+    return ComparisonResult::Same;
   // If exactly one is true, then they're different, no reason to check whether
   // they're definitely empty.
-  if (MustNonEmpty1 || MustNonEmpty2) return ComparisonResult::Different;
+  if (MustNonEmpty1 || MustNonEmpty2)
+    return ComparisonResult::Different;
   // Check if they're both definitely empty.
   return (isEmptyOptional(Val1, Env1) && isEmptyOptional(Val2, Env2))
              ? ComparisonResult::Same
              : ComparisonResult::Different;
 }
 
 bool UncheckedOptionalAccessModel::merge(QualType Type, const Value &Val1,
                                          const Environment &Env1,