[clang] [clang][dataflow] Add a lattice to help cache const accessor methods (PR #111006)

[Nico Weber via cfe-commits]

================
@@ -0,0 +1,218 @@
+//===-- CachedConstAccessorsLattice.h ---------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the lattice mixin that additionally maintains a cache of
+// stable method call return values to model const accessor member functions.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CACHED_CONST_ACCESSORS_LATTICE_H
+#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CACHED_CONST_ACCESSORS_LATTICE_H
+
+#include "clang/AST/Expr.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
+#include "clang/Analysis/FlowSensitive/Value.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
+
+namespace clang {
+namespace dataflow {
+
+/// A mixin for a lattice that additionally maintains a cache of stable method
+/// call return values to model const accessors methods. When a non-const method
+/// is called, the cache should be cleared causing the next call to a const
+/// method to be considered a different value. NOTE: The user is responsible for
+/// clearing the cache.
+///
+/// For example:
+///
+/// class Bar {
+/// public:
+///   const std::optional<Foo>& getFoo() const;
+///   void clear();
+/// };
+//
+/// void func(Bar& s) {
+///   if (s.getFoo().has_value()) {
+///     use(s.getFoo().value()); // safe (checked earlier getFoo())
+///     s.clear();
+///     use(s.getFoo().value()); // unsafe (invalidate cache for s)
+///   }
+/// }
+template <typename Base> class CachedConstAccessorsLattice : public Base {
+public:
+  using Base::Base; // inherit all constructors
+
+  /// Creates or returns a previously created `Value` associated with a const
+  /// method call `obj.getFoo()` where `RecordLoc` is the
+  /// `RecordStorageLocation` of `obj`.
+  /// Returns nullptr if unable to find or create a value.
+  ///
+  /// Requirements:
+  ///
+  ///  - `CE` should return a value (not a reference or record type)
+  Value *
+  getOrCreateConstMethodReturnValue(const RecordStorageLocation &RecordLoc,
+                                    const CallExpr *CE, Environment &Env);
+
+  /// Creates or returns a previously created `StorageLocation` associated with
+  /// a const method call `obj.getFoo()` where `RecordLoc` is the
+  /// `RecordStorageLocation` of `obj`.
+  ///
+  /// The callback `Initialize` runs on the storage location if newly created.
+  /// Returns nullptr if unable to find or create a value.
+  ///
+  /// Requirements:
+  ///
+  ///  - `CE` should return a location (GLValue or a record type).
+  StorageLocation *getOrCreateConstMethodReturnStorageLocation(
+      const RecordStorageLocation &RecordLoc, const CallExpr *CE,
+      Environment &Env, llvm::function_ref<void(StorageLocation &)> Initialize);
+
+  void clearConstMethodReturnValues(const RecordStorageLocation &RecordLoc) {
+    ConstMethodReturnValues.erase(&RecordLoc);
+  }
+
+  void clearConstMethodReturnStorageLocations(
+      const RecordStorageLocation &RecordLoc) {
+    ConstMethodReturnStorageLocations.erase(&RecordLoc);
+  }
+
+  bool operator==(const CachedConstAccessorsLattice &Other) const {
+    return Base::operator==(Other);
+  }
+
+  LatticeJoinEffect join(const CachedConstAccessorsLattice &Other);
+
+private:
+  // Maps a record storage location and const method to the value to return
+  // from that const method.
+  using ConstMethodReturnValuesType =
+      llvm::SmallDenseMap<const RecordStorageLocation *,
+                          llvm::SmallDenseMap<const FunctionDecl *, Value *>>;
+  ConstMethodReturnValuesType ConstMethodReturnValues;
+
+  // Maps a record storage location and const method to the record storage
+  // location to return from that const method.
+  using ConstMethodReturnStorageLocationsType = llvm::SmallDenseMap<
+      const RecordStorageLocation *,
+      llvm::SmallDenseMap<const FunctionDecl *, StorageLocation *>>;
+  ConstMethodReturnStorageLocationsType ConstMethodReturnStorageLocations;
+};
+
+namespace internal {
----------------
nico wrote:

This breaks building on Windows (arguably by exposing an existing bug):

http://45.33.8.238/win/95986/step_3.txt (scroll to the very bottom):

```
../../clang/include\clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h(157,29): error: reference to 'internal' is ambiguous
  157 |   ConstMethodReturnValues = internal::joinConstMethodMap<Value>(
      |                             ^
../../clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp(1083,31): note: in instantiation of member function 'clang::dataflow::DataflowAnalysis<clang::dataflow::UncheckedOptionalAccessModel, clang::dataflow::CachedConstAccessorsLattice<clang::dataflow::NoopLattice>>::joinTypeErased' requested here
 1083 | UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(ASTContext &Ctx,
      |                               ^
../../clang/include\clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h(109,11): note: candidate found by name lookup is 'clang::dataflow::internal'
  109 | namespace internal {
      |           ^
../../clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp(205,1): note: candidate found by name lookup is 'clang::dataflow::(anonymous namespace)::internal'
  205 | AST_MATCHER_P(CXXMemberCallExpr, publicReceiverType,
      | ^
../../clang/include\clang/ASTMatchers/ASTMatchersMacros.h(131,3): note: expanded from macro 'AST_MATCHER_P'
  131 |   AST_MATCHER_P_OVERLOAD(Type, DefineMatcher, ParamType, Param, 0)
      |   ^
../../clang/include\clang/ASTMatchers/ASTMatchersMacros.h(135,13): note: expanded from macro 'AST_MATCHER_P_OVERLOAD'
  135 |   namespace internal {                                                         \
      |             ^
```

Please take a look and revert for now if it takes a while to fix.

https://github.com/llvm/llvm-project/pull/111006