[clang] [clang][dataflow] Don't clear cached field state if field is const (PR #113698)

[Jan Voung via cfe-commits]

https://github.com/jvoung created https://github.com/llvm/llvm-project/pull/113698

... in the unchecked optional access model.


>From 8b955ead1e7445a7226f51078ba2d05e52f15c23 Mon Sep 17 00:00:00 2001
From: Jan Voung <jvoung at gmail.com>
Date: Fri, 25 Oct 2024 14:56:49 +0000
Subject: [PATCH] [clang][dataflow] Don't clear cached field state if field is
 const

... in the unchecked optional access model.
---
 .../Models/UncheckedOptionalAccessModel.cpp   |  8 +++++--
 .../UncheckedOptionalAccessModelTest.cpp      | 23 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index b0bd8274405d02..31ae2b94f5b617 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -601,10 +601,14 @@ void handleNonConstMemberCall(const CallExpr *CE,
                               dataflow::RecordStorageLocation *RecordLoc,
                               const MatchFinder::MatchResult &Result,
                               LatticeTransferState &State) {
-  // When a non-const member function is called, reset some state.
   if (RecordLoc != nullptr) {
+    // When a non-const member function is called, clear all (non-const)
+    // optional fields of the receiver. Const-qualified fields can't be
+    // changed (at least, not without UB).
     for (const auto &[Field, FieldLoc] : RecordLoc->children()) {
-      if (isSupportedOptionalType(Field->getType())) {
+      QualType FieldType = Field->getType();
+      if (!FieldType.isConstQualified() &&
+          isSupportedOptionalType(Field->getType())) {
         auto *FieldRecordLoc = cast_or_null<RecordStorageLocation>(FieldLoc);
         if (FieldRecordLoc) {
           setHasValue(*FieldRecordLoc, State.Env.makeAtomicBoolValue(),
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 22fe347c425593..5b64eaca0e10d3 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2167,7 +2167,7 @@ TEST_P(UncheckedOptionalAccessTest, OptionalReturnedFromFuntionCall) {
   )");
 }
 
-TEST_P(UncheckedOptionalAccessTest, OptionalFieldModified) {
+TEST_P(UncheckedOptionalAccessTest, NonConstMethodMayClearOptionalField) {
   ExpectDiagnosticsFor(
       R"(
     #include "unchecked_optional_access_test.h"
@@ -2187,6 +2187,27 @@ TEST_P(UncheckedOptionalAccessTest, OptionalFieldModified) {
   )");
 }
 
+TEST_P(UncheckedOptionalAccessTest,
+       NonConstMethodMayNotClearConstOptionalField) {
+  ExpectDiagnosticsFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+
+    struct Foo {
+      const $ns::$optional<std::string> opt;
+      void clear();
+    };
+
+    void target(Foo& foo) {
+      if (foo.opt) {
+        foo.opt.value();
+        foo.clear();
+        foo.opt.value();
+      }
+    }
+  )");
+}
+
 TEST_P(UncheckedOptionalAccessTest, StdSwap) {
   ExpectDiagnosticsFor(
       R"(