[PATCH] D130726: [clang][dataflow] Handle multiple context-sensitive calls to the same function

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.


================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3963
 
+TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) {
+  std::string Code = R"(
----------------
samestep wrote:
> ymandel wrote:
> > Are there more scenarios testable at this point? e.g
> > 1. 2 layers of callees
> > 2. more than one line of code inside the body?
> > 3. one than one CFG block in the body?
> > 
> > If so, please add tests for those that are supported.
> 1. This currently isn't supported.
> 2. This should work.
> 3. This should work.
> 
> For (2) and (3), should I add those tests now, or do that in a followup patch?
> 1. This currently isn't supported.
What's limiting us here? Just curious, not asking you to update the comments, etc.

> 2. This should work.
> 3. This should work.
> 
> For (2) and (3), should I add those tests now, or do that in a followup patch?

may as well add them now. thx


================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3963
 
+TEST(TransferTest, ContextSensitiveSetBothTrueAndFalse) {
+  std::string Code = R"(
----------------
ymandel wrote:
> samestep wrote:
> > ymandel wrote:
> > > Are there more scenarios testable at this point? e.g
> > > 1. 2 layers of callees
> > > 2. more than one line of code inside the body?
> > > 3. one than one CFG block in the body?
> > > 
> > > If so, please add tests for those that are supported.
> > 1. This currently isn't supported.
> > 2. This should work.
> > 3. This should work.
> > 
> > For (2) and (3), should I add those tests now, or do that in a followup patch?
> > 1. This currently isn't supported.
> What's limiting us here? Just curious, not asking you to update the comments, etc.
> 
> > 2. This should work.
> > 3. This should work.
> > 
> > For (2) and (3), should I add those tests now, or do that in a followup patch?
> 
> may as well add them now. thx
> 1. This currently isn't supported.
> 2. This should work.
> 3. This should work.
> 
> For (2) and (3), should I add those tests now, or do that in a followup patch?




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130726/new/

https://reviews.llvm.org/D130726