samestep added inline comments.
================
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:114
+ ///
+ /// `return` must not be assigned a storage location.
+ void setReturnStorageLocation(StorageLocation &Loc) {
----------------
li.zhe.hua wrote:
> Fix this as well? A reader shouldn't need to root around in private
> implementation details to understand the requirements for calling a function.
Could you clarify what you mean? I was simply copying the signature and
docstring from `setThisPointeeStorageLocation`.
================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:221
+
// FIXME: Currently this only works if the callee is never a method and the
// same callee is never analyzed from multiple separate callsites. To
----------------
li.zhe.hua wrote:
> I'm a little unclear on what "this" is. Is it this entire function, or just
> the call to `getDirectCallee()`? Can this comment be moved somewhere more
> appropriate, and specifically, so it is touching the code that is most
> relevant to it? It is currently floating in the middle of the function, and
> it's unclear to me why new code is being added above it vs. below it.
Yes, this comment is not meant to be present in this patch; I am trying to
update the parent patch to remove this comment, but am currently unable to do
export it to Phabricator for technical reasons. I should be able to do that
tomorrow.
================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3908
+ Var = true;
+ return;
+ }
----------------
li.zhe.hua wrote:
> Why is this change to the test necessary?
This is mentioned in the patch description (updated earlier today); it's not
necessary, but I added it to get a bit of extra coverage for some cases in the
`VisitReturnStmt` method this patch adds.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130600/new/
https://reviews.llvm.org/D130600
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
