[clang] 0086a35 - [clang][dataflow] Fix bug in optional-checker's handling of nullopt constructor.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2023-01-03T21:57:39Z
New Revision: 0086a3555ac6cd76bb637252a0ba17c06c9b869b

URL: https://github.com/llvm/llvm-project/commit/0086a3555ac6cd76bb637252a0ba17c06c9b869b
DIFF: https://github.com/llvm/llvm-project/commit/0086a3555ac6cd76bb637252a0ba17c06c9b869b.diff

LOG: [clang][dataflow] Fix bug in optional-checker's handling of nullopt constructor.

Currently, the checker only recognizes the nullopt constructor when it is called
without sugar, resulting in a crash in the (rare) case where it has been wrapped
in sugar. This relaxes the constraint by checking the constructor decl directly
(which always contains the same, desugared form) rather than the construct
expression (where the spelling depends on the context).

Differential Revision: https://reviews.llvm.org/D140921

Added: 
    

Modified: 
    clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
    clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 07ec16c9cc6ef..10b9866d5c23c 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -22,6 +22,7 @@
 #include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/StringRef.h"
@@ -100,8 +101,10 @@ auto inPlaceClass() {
 }
 
 auto isOptionalNulloptConstructor() {
-  return cxxConstructExpr(hasOptionalType(), argumentCountIs(1),
-                          hasArgument(0, hasNulloptType()));
+  return cxxConstructExpr(
+      hasOptionalType(),
+      hasDeclaration(cxxConstructorDecl(parameterCountIs(1),
+                                        hasParameter(0, hasNulloptType()))));
 }
 
 auto isOptionalInPlaceConstructor() {
@@ -452,6 +455,7 @@ void assignOptionalValue(const Expr &E, Environment &Env,
 BoolValue &valueOrConversionHasValue(const FunctionDecl &F, const Expr &E,
                                      const MatchFinder::MatchResult &MatchRes,
                                      LatticeTransferState &State) {
+  assert(F.getTemplateSpecializationArgs() != nullptr);
   assert(F.getTemplateSpecializationArgs()->size() > 0);
 
   const int TemplateParamOptionalWrappersCount = countOptionalWrappers(

diff  --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 1fcede5d62865..2d108e8d24832 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1498,6 +1498,23 @@ TEST_P(UncheckedOptionalAccessTest, NulloptConstructor) {
   )");
 }
 
+TEST_P(UncheckedOptionalAccessTest, NulloptConstructorWithSugaredType) {
+  ExpectDiagnosticsFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+    template <typename T>
+    using wrapper = T;
+
+    template <typename T>
+    wrapper<T> wrap(T);
+
+    void target() {
+      $ns::$optional<int> opt(wrap($ns::nullopt));
+      opt.value(); // [[unsafe]]
+    }
+  )");
+}
+
 TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
   ExpectDiagnosticsFor(R"(
     #include "unchecked_optional_access_test.h"