Skip to content

Commit f9b8cba

Browse files
committed
[clang][dataflow] Add matchers for smart pointer accessors to be cached
This is part 1 of caching for smart pointer accessors, building on top of the CachedConstAccessorsLattice, which caches "normal" accessors. Smart pointer accessors are a bit different in that they may: - have aliases to access the same underlying data (but potentially returning slightly different types like `&` vs `*`). Within a "checked" sequence users may mix uses of the different aliases and the check should apply to any of the spellings. - may have non-const overloads in addition to the const version, where the non-const doesn't actually modify the container Part 2 will follow and add transfer functions utilities. It will also add a user UncheckedOptionalAccessModel. We'd seen false positives when nesting StatusOr<optional<T>> and optional<StatusOr<T>>, etc. which this can help address.
1 parent 3c3094b commit f9b8cba

File tree

5 files changed

+393
-0
lines changed

5 files changed

+393
-0
lines changed
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//===-- SmartPointerAccessorCaching.h ---------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// This file defines utilities to help cache accessors for smart pointer
10+
// like objects.
11+
//
12+
// These should be combined with CachedConstAccessorsLattice.
13+
// Beyond basic const accessors, smart pointers may have the following two
14+
// additional issues:
15+
//
16+
// 1) There may be multiple accessors for the same underlying object, e.g.
17+
// `operator->`, `operator*`, and `get`. Users may use a mixture of these
18+
// accessors, so the cache should unify them.
19+
//
20+
// 2) There may be non-const overloads of accessors. They are still safe to
21+
// cache, as they don't modify the container object.
22+
//===----------------------------------------------------------------------===//
23+
24+
#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_SMARTPOINTERACCESSORCACHING_H
25+
#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_SMARTPOINTERACCESSORCACHING_H
26+
27+
#include <cassert>
28+
29+
#include "clang/AST/Decl.h"
30+
#include "clang/AST/Stmt.h"
31+
#include "clang/ASTMatchers/ASTMatchers.h"
32+
33+
namespace clang::dataflow {
34+
35+
/// Matchers:
36+
/// For now, these match on any class with an `operator*` or `operator->`
37+
/// where the return types have a similar shape as std::unique_ptr
38+
/// and std::optional.
39+
///
40+
/// - `*` returns a reference to a type `T`
41+
/// - `->` returns a pointer to `T`
42+
/// - `get` returns a pointer to `T`
43+
/// - `value` returns a reference `T`
44+
///
45+
/// (1) The `T` should all match across the accessors (ignoring qualifiers).
46+
///
47+
/// (2) The specific accessor used in a call isn't required to be const,
48+
/// but the class must have a const overload of each accessor.
49+
///
50+
/// For now, we don't have customization to ignore certain classes.
51+
/// For example, if writing a ClangTidy check for `std::optional`, these
52+
/// would also match `std::optional`. In order to have special handling
53+
/// for `std::optional`, we assume the (Matcher, TransferFunction) case
54+
/// with custom handling is ordered early so that these generic cases
55+
/// do not trigger.
56+
ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeOperatorStar();
57+
ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeOperatorArrow();
58+
ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeValueMethodCall();
59+
ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeGetMethodCall();
60+
61+
} // namespace clang::dataflow
62+
63+
#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_SMARTPOINTERACCESSORCACHING_H

clang/lib/Analysis/FlowSensitive/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add_clang_library(clangAnalysisFlowSensitive
1010
Logger.cpp
1111
RecordOps.cpp
1212
SimplifyConstraints.cpp
13+
SmartPointerAccessorCaching.cpp
1314
Transfer.cpp
1415
TypeErasedDataflowAnalysis.cpp
1516
Value.cpp
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
#include "clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h"
2+
3+
#include "clang/AST/CanonicalType.h"
4+
#include "clang/AST/DeclCXX.h"
5+
#include "clang/ASTMatchers/ASTMatchers.h"
6+
#include "clang/Basic/OperatorKinds.h"
7+
8+
namespace clang::dataflow {
9+
10+
namespace {
11+
12+
using ast_matchers::callee;
13+
using ast_matchers::cxxMemberCallExpr;
14+
using ast_matchers::cxxMethodDecl;
15+
using ast_matchers::cxxOperatorCallExpr;
16+
using ast_matchers::hasName;
17+
using ast_matchers::hasOverloadedOperatorName;
18+
using ast_matchers::ofClass;
19+
using ast_matchers::parameterCountIs;
20+
using ast_matchers::pointerType;
21+
using ast_matchers::referenceType;
22+
using ast_matchers::returns;
23+
24+
bool hasSmartPointerClassShape(const CXXRecordDecl &RD, bool &HasGet,
25+
bool &HasValue) {
26+
// We may want to cache this search, but in current profiles it hasn't shown
27+
// up as a hot spot (possibly because there aren't many hits, relatively).
28+
bool HasArrow = false;
29+
bool HasStar = false;
30+
CanQualType StarReturnType, ArrowReturnType, GetReturnType, ValueReturnType;
31+
for (const auto *MD : RD.methods()) {
32+
// We only consider methods that are const and have zero parameters.
33+
// It may be that there is a non-const overload for the method, but
34+
// there should at least be a const overload as well.
35+
if (!MD->isConst() || MD->getNumParams() != 0) {
36+
continue;
37+
}
38+
if (MD->getOverloadedOperator() == OO_Star &&
39+
MD->getReturnType()->isReferenceType()) {
40+
HasStar = true;
41+
StarReturnType = MD->getReturnType()
42+
.getNonReferenceType()
43+
->getCanonicalTypeUnqualified();
44+
} else if (MD->getOverloadedOperator() == OO_Arrow &&
45+
MD->getReturnType()->isPointerType()) {
46+
HasArrow = true;
47+
ArrowReturnType =
48+
MD->getReturnType()->getPointeeType()->getCanonicalTypeUnqualified();
49+
} else {
50+
IdentifierInfo *II = MD->getIdentifier();
51+
if (II == nullptr)
52+
continue;
53+
if (II->isStr("get") && MD->getReturnType()->isPointerType()) {
54+
HasGet = true;
55+
GetReturnType = MD->getReturnType()
56+
->getPointeeType()
57+
->getCanonicalTypeUnqualified();
58+
} else if (II->isStr("value") && MD->getReturnType()->isReferenceType()) {
59+
HasValue = true;
60+
ValueReturnType = MD->getReturnType()
61+
.getNonReferenceType()
62+
->getCanonicalTypeUnqualified();
63+
}
64+
}
65+
}
66+
67+
if (!HasStar || !HasArrow || StarReturnType != ArrowReturnType)
68+
return false;
69+
HasGet = HasGet && (GetReturnType == StarReturnType);
70+
HasValue = HasValue && (ValueReturnType == StarReturnType);
71+
return true;
72+
}
73+
74+
} // namespace
75+
} // namespace clang::dataflow
76+
77+
// AST_MATCHER macros create an "internal" namespace, so we put it in
78+
// its own anonymous namespace instead of in clang::dataflow.
79+
namespace {
80+
81+
AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithGet) {
82+
bool HasGet = false;
83+
bool HasValue = false;
84+
bool HasStarAndArrow =
85+
clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
86+
return HasStarAndArrow && HasGet;
87+
}
88+
89+
AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithValue) {
90+
bool HasGet = false;
91+
bool HasValue = false;
92+
bool HasStarAndArrow =
93+
clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
94+
return HasStarAndArrow && HasValue;
95+
}
96+
97+
AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithGetOrValue) {
98+
bool HasGet = false;
99+
bool HasValue = false;
100+
bool HasStarAndArrow =
101+
clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
102+
return HasStarAndArrow && (HasGet || HasValue);
103+
}
104+
105+
} // namespace
106+
107+
namespace clang::dataflow {
108+
109+
ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeOperatorStar() {
110+
return cxxOperatorCallExpr(
111+
hasOverloadedOperatorName("*"),
112+
callee(cxxMethodDecl(parameterCountIs(0), returns(referenceType()),
113+
ofClass(smartPointerClassWithGetOrValue()))));
114+
}
115+
116+
ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeOperatorArrow() {
117+
return cxxOperatorCallExpr(
118+
hasOverloadedOperatorName("->"),
119+
callee(cxxMethodDecl(parameterCountIs(0), returns(pointerType()),
120+
ofClass(smartPointerClassWithGetOrValue()))));
121+
}
122+
ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeValueMethodCall() {
123+
return cxxMemberCallExpr(callee(
124+
cxxMethodDecl(parameterCountIs(0), returns(referenceType()),
125+
hasName("value"), ofClass(smartPointerClassWithValue()))));
126+
}
127+
128+
ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeGetMethodCall() {
129+
return cxxMemberCallExpr(callee(
130+
cxxMethodDecl(parameterCountIs(0), returns(pointerType()), hasName("get"),
131+
ofClass(smartPointerClassWithGet()))));
132+
}
133+
134+
} // namespace clang::dataflow

clang/unittests/Analysis/FlowSensitive/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ add_clang_unittest(ClangAnalysisFlowSensitiveTests
2121
SignAnalysisTest.cpp
2222
SimplifyConstraintsTest.cpp
2323
SingleVarConstantPropagationTest.cpp
24+
SmartPointerAccessorCachingTest.cpp
2425
TestingSupport.cpp
2526
TestingSupportTest.cpp
2627
TransferBranchTest.cpp

0 commit comments

Comments
 (0)