sgatev 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:
> samestep wrote:
> > 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`.
> You've marked `return` in backticks. There is no parameter named `return` and
> it is unclear what `return` refers to. My best guess is that this is a typo
> of `ReturnLoc`, which is a private data member. So this is a public interface
> with a requirement that a private data member has some property. This should
> instead reframe the requirement as properties from the external reader's
> perspective.
That was my guess initially too, but my next best guess is that `return` in
backticks stands for the keyword/AST node. In any case, let's make it less
ambiguous and let's not add requirements based on implementation details. How
about: `The return value must not be assigned a storage location.`?
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:338-339
+ if (Loc == nullptr) {
+ // The outermost context does not set a storage location for `return`, so
+ // in that case we just ignore `return` statements.
+ return;
----------------
samestep wrote:
> sgatev wrote:
> > Let's make this a FIXME to set a storage location for the outermost context
> > too.
> @sgatev I could add a `FIXME` for that, or I could just do it in this same
> patch; do you have a preference between those two options?
Same patch works!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130600/new/
https://reviews.llvm.org/D130600
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
