aaron.ballman added inline comments.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:17-19
+// Because the analysis revolves around variables and their types, we'll need
to
+// track uses of variables (aka DeclRefExprs).
+using DeclUseList = SmallVector<const DeclRefExpr *, 1>;
----------------
NoQ wrote:
> aaron.ballman wrote:
> > Do we have to care about data member accesses (which would be a
> > `MemberExpr` and not a `DeclRefExpr`)?
> >
> > Also, because this shows up in interfaces, it should probably be a
> > `SmallVectorImpl` so the size doesn't matter.
> > Do we have to care about data member accesses (which would be a
> > `MemberExpr` and not a `DeclRefExpr`)?
>
> Unlikely. Auto-fixing member variables without breaking source or binary
> compatibility is next to impossible.
>
> > Also, because this shows up in interfaces, it should probably be a
> > SmallVectorImpl so the size doesn't matter.
>
> This doesn't seem to work on return types, which is where these classes show
> up in my case (`SmallVectorImpl` has deleted copy-move constructors so it's
> only suitable for passing by reference).
>>Do we have to care about data member accesses (which would be a MemberExpr
>>and not a DeclRefExpr)?
> Unlikely. Auto-fixing member variables without breaking source or binary
> compatibility is next to impossible.
Okie dokie!
>> Also, because this shows up in interfaces, it should probably be a
>> SmallVectorImpl so the size doesn't matter.
> This doesn't seem to work on return types, which is where these classes show
> up in my case (SmallVectorImpl has deleted copy-move constructors so it's
> only suitable for passing by reference).
Ahhh I had forgotten that we didn't allow it as a move-only type.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:266-271
+ for (const Decl *D: DS->decls()) {
+ if (const auto *VD = dyn_cast<VarDecl>(D)) {
+ assert(Defs.count(VD) == 0 && "Definition already discovered!");
+ Defs[VD] = DS;
+ }
+ }
----------------
NoQ wrote:
> aaron.ballman wrote:
> > Use a `specific_decl_iterator<VarDecl>` instead?
> Hmm `DeclStmt` doesn't seem to provide a neat accessor, would you like me to
> add it?
Eh, I don't insist. Feel free to do it as an NFC change after landing this
patch, if you want to.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138253/new/
https://reviews.llvm.org/D138253
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
