ziqingluo-90 added inline comments.
================
Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h:25
+
+class VariableGroupsManager {
+public:
----------------
Use the `VariableGroupsManager` class as an interface to clients. It hides the
details about how groups are implemented (which may involve several
containers).
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1143
+ // order is deterministic:
+ CompareNode<VarDecl>>
+ byVar;
----------------
To make the order of variables in every group deterministic.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2179
+ if (FixItsForVariable.count(GrpMate))
+ FinalFixItsForVariable[Var].insert(FinalFixItsForVariable[Var].end(),
+ FixItsForVariable[GrpMate].begin(),
----------------
Instead of calling `fixVariable` to generate fix-its for group mates, directly
copying those fix-its from the `FixItsForVariable` container.
At this point, `FixItsForVariable` is complete. That is, it maps every
variable to its own fix-its (excluding fix-its of group mates).
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2198
+// Manages variable groups:
+class VariableGroupsManagerImpl : public VariableGroupsManager {
+ const std::vector<VarGrpTy> Groups;
----------------
An implementation of `VariableGroupsManager`: keep a unique copy of each group
in `Groups` and use their indexes for access.
Note that `getGroupOfVar` assumes that `Var` must in the map. This is
guaranteed by the changed made in [[ https://reviews.llvm.org/D155524 |
D155524 ]]---only "Fixables" associated to variables in the final graph(es)
will stay and be fixed. The key set of the `VarGrpMap` is the set of
variables in the final graph(es). Therefore, after the graph(es) are computed
and "Fixables" are pruned, `VarGrpMap` is the variable domain that we will
deal with.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2309
// Fixpoint iteration for pointer assignments
- using DepMapTy = DenseMap<const VarDecl *, std::set<const VarDecl *>>;
+ using DepMapTy = DenseMap<const VarDecl *, llvm::SetVector<const VarDecl *>>;
DepMapTy DependenciesMap{};
----------------
Use `SetVector` to make order deterministic.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2369
- for (const VarDecl * V : VarGroup) {
- if (UnsafeOps.byVar.find(V) != UnsafeOps.byVar.end()) {
- VariableGroupsMap[V] = VarGroup;
----------------
I removed this guard so that `VarGrpMap` is complete---it has an entry for
every variable that we need to deal with later.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2394
+ for (const VarDecl *V : VarGroup) {
+ VarGrpMap[V] = GrpIdx;
}
----------------
Mapping variables to group indexes instead of groups themselves to avoid copies.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2168
+ // itself:
+ std::string listVariableGroupAsString(
+ const VarDecl *VD, const ArrayRef<const VarDecl *> &VarGroupForVD) const
{
----------------
Factor this procedure out to a method for easy re-use later.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156474/new/
https://reviews.llvm.org/D156474
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
