ziqingluo-90 added inline comments.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2586
#endif
it = FixablesForAllVars.byVar.erase(it);
} else if (Tracker.hasUnclaimedUses(it->first)) {
----------------
We cannot fix reference type variable declarations for now.
Besides, it seems that reference type is invisible to `Expr`s. For example,
```
void foo(int * &p) {
p[5]; // the type of the DRE `p` is a pointer-to-int instead of a
reference
}
```
So we ignore such variables explicitly here.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2719
+ Strategy NaiveStrategy = getNaiveStrategy(
+ llvm::make_range(VisitedVars.begin(), VisitedVars.end()));
VariableGroupsManagerImpl VarGrpMgr(Groups, VarGrpMap, GrpsUnionForParms);
----------------
t-rasmud wrote:
> ziqingluo-90 wrote:
> > `VisitedVars` are the set of variables on the computed graph. The previous
> > `UnsafeVars` is a superset of it, including safe variables that have
> > `FixableGadget`s discovered. We do not want to assign strategies other
> > than `Won't fix` to those safe variables.
> Would it make more sense (make the code more readable) if we renamed
> `UnsafeVars` to say, `FixableVars`?
Oh, I just realized that `UnsafeVars` is now an unused variable. So let's
remove it. `FixableVars` would be a better name for sure, since it is in fact
the key set of `FixablesForAllVars.byVar`.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2719
+ Strategy NaiveStrategy = getNaiveStrategy(
+ llvm::make_range(VisitedVars.begin(), VisitedVars.end()));
VariableGroupsManagerImpl VarGrpMgr(Groups, VarGrpMap, GrpsUnionForParms);
----------------
ziqingluo-90 wrote:
> t-rasmud wrote:
> > ziqingluo-90 wrote:
> > > `VisitedVars` are the set of variables on the computed graph. The
> > > previous `UnsafeVars` is a superset of it, including safe variables that
> > > have `FixableGadget`s discovered. We do not want to assign strategies
> > > other than `Won't fix` to those safe variables.
> > Would it make more sense (make the code more readable) if we renamed
> > `UnsafeVars` to say, `FixableVars`?
> Oh, I just realized that `UnsafeVars` is now an unused variable. So let's
> remove it. `FixableVars` would be a better name for sure, since it is in fact
> the key set of `FixablesForAllVars.byVar`.
To correct myself, `VisitedVars` are the variables In the graph but it may not
be a subset of "fixable variables". `VisitedVars` may contain variables that
is not fixable, i.e., variables having no `FixableGadget` associated with. We
should leave those unfixable variables being WON'T FIX.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157441/new/
https://reviews.llvm.org/D157441
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
