samestep added inline comments.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:138
///
+ /// `Call` must be either a `CallExpr` or a `CXXConstructExpr`.
+ ///
----------------
sgatev wrote:
> How about we define overloads that take these types instead of taking an
> `Expr` here? This should remove the need for type-checking and guarding
> against bad input in the implementation. `transferInlineCall` can be a
> template if necessary.
Hmm I guess we could; is there much of a benefit to doing this templated?
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:678-684
+ if (const auto *NonConstructExpr = dyn_cast<CallExpr>(S)) {
+ // Note that it is important for the storage location of `S` to be set
+ // before `pushCall`, because the latter uses it to set the storage
+ // location for `return`.
+ auto &ReturnLoc = Env.createStorageLocation(*S);
+ Env.setStorageLocation(*S, ReturnLoc);
+ }
----------------
ymandel wrote:
> Why can't this stay in `VisitCallExpr`?
I had it there in an earlier version of this patch; it was causing tests to
fail (the `SelfReferential*` ones, if I remember correctly).
================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:4371
+TEST(TransferTest, ContextSensitiveConstructorBody) {
+ std::string Code = R"(
----------------
ymandel wrote:
> What about a default constructor, including when there are field initializers
> like:
> ```
> class MyClass {
> public:
> MyClass() = default;
>
> bool MyField = true;
> };
> ```
>
> Should we expect to handle that correctly? If so, can you add some tests?
Good idea! I'll add a test for this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131438/new/
https://reviews.llvm.org/D131438
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
