-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang][dataflow] Add a lattice to help cache const accessor methods #111006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
f82e63e
4885d65
10e69fd
5578e31
b23b67b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -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 a | ||||||
|
||||||
| /// Creates or returns a previously created `StorageLocation` associated a | |
| /// Creates or returns a previously created `StorageLocation` associated with a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah -- Taking a look!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Here's a build that needs less scrolling to find the diag: http://45.33.8.238/win/95994/step_3.txt)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Created #113601
Though is there a way to validate this, similar to this clang-cl compile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!