[PATCH] D127434: [clang][dataflow] Match call return via hasTypeCurrently the unchecked-optional-access model fails on this example: #include <memory> #include <optional> void foo() { std::unique_ptr<std::optional<float>> x; *x = std...

[Sam Estep via Phabricator via cfe-commits]

samestep created this revision.
samestep added a reviewer: ymandel.
Herald added subscribers: tschuett, steakhal, xazax.hun.
Herald added a project: All.
samestep requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...::nullopt;

  }

You can verify the failure by saving the file as `foo.cpp` and running this
command:

  clang-tidy -checks='-*,bugprone-unchecked-optional-access' \
    foo.cpp -- -std=c++17

The failing `assert` is in the `transferAssignment` function of the
`UncheckedOptionalAccessModel.cpp` file:

  assert(OptionalLoc != nullptr);

The symptom can be treated by replacing that `assert` with an early `return`:

  if (OptionalLoc == nullptr)
    return;

That would be better anyway since we cannot expect to always cover all possible
LHS expressions, but it is out of scope for this patch and left as a followup.

Note that the failure did not occur on this very similar example:

  #include <optional>
  
  template <typename T>
  struct smart_ptr {
    T& operator*() &;
    T* operator->();
  };
  
  void foo() {
    smart_ptr<std::optional<float>> x;
    *x = std::nullopt;
  }

The difference is caused by the `isCallReturningOptional` matcher, which was
previously checking the `functionDecl` of the `callee`. This patch changes it to
instead use `hasType` directly on the call expression, fixing the failure for
the `std::unique_ptr` example above.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127434

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


Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -165,8 +165,11 @@
 }
 
 auto isCallReturningOptional() {
-  return callExpr(callee(functionDecl(returns(anyOf(
-      optionalOrAliasType(), referenceType(pointee(optionalOrAliasType())))))));
+  // We must extract this to a variable to specify its type, because otherwise
+  // multiple overloads of `hasType` match.
+  ast_matchers::internal::Matcher<Type> typeMatcher = anyOf(
+      optionalOrAliasType(), referenceType(pointee(optionalOrAliasType())));
+  return callExpr(hasType(typeMatcher));
 }
 
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D127434.435643.patch
Type: text/x-patch
Size: 891 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220609/7b0b5853/attachment.bin>