[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

eugenis added a comment. Sorry but I had to revert this because of the conflicts with another revert: https://reviews.llvm.org/D131065 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG000c8fef86ab: [clang][dataflow] Analyze constructor bodies (authored by samestep). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep updated this revision to Diff 451445. samestep added a comment. Address Stanislav's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 Files:

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:379 + /// Shared implementation of `pushCall` overloads. + void pushCallInternal(const FunctionDecl *FuncDecl, +ArrayRef Args);

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

sgatev accepted this revision. sgatev added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:379 + /// Shared implementation of `pushCall` overloads. + void pushCallInternal(const FunctionDecl *FuncDecl, +

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:224 // FIXME: Support references here. - Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference); + ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep updated this revision to Diff 451267. samestep added a comment. Remove Call parameter from pushCallInternal method Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 Files:

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

ymandel added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:224 // FIXME: Support references here. - Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference); + ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun accepted this revision. xazax.hun added a comment. Nice! We went from branches in the loop to branches before the loop first. And now the branches are at compile time during overload resolution. I love it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep updated this revision to Diff 451265. samestep added a comment. Pull type-specific logic into overloads Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 Files:

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:219-245 // FIXME: Support references here. - Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference); + ReturnLoc = getStorageLocation(*Call,

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:219-245 // FIXME: Support references here. - Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference); + ReturnLoc = getStorageLocation(*Call,

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun accepted this revision. xazax.hun added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 ___ cfe-commits mailing list

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

ymandel added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:219-245 // FIXME: Support references here. - Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference); + ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep updated this revision to Diff 451259. samestep added a comment. Use ArrayRef Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 Files:

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep added a comment. In D131438#3710697 , @xazax.hun wrote: > I feel like this is a repeated pattern. The CSA solved a very similar issue > by introducing the CallEvent class hierarchy. I also remember seeing many > disparate code snippets

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:248-258 + for (unsigned ArgIndex = 0; ArgIndex < NumArgs; ++ParamIt, ++ArgIndex) { assert(ParamIt != FuncDecl->param_end()); -const Expr *Arg = *ArgIt; -auto

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:248-258 + for (unsigned ArgIndex = 0; ArgIndex < NumArgs; ++ParamIt, ++ArgIndex) { assert(ParamIt != FuncDecl->param_end()); -const Expr *Arg = *ArgIt; -auto

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:248-258 + for (unsigned ArgIndex = 0; ArgIndex < NumArgs; ++ParamIt, ++ArgIndex) { assert(ParamIt != FuncDecl->param_end()); -const Expr *Arg = *ArgIt; -auto

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

xazax.hun added a comment. I feel like this is a repeated pattern. The CSA solved a very similar issue by introducing the CallEvent class hierarchy. I also remember seeing many disparate code snippets littered throughout the clang codebase that tries to deal with the problem of not having

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep updated this revision to Diff 451233. samestep added a comment. Narrow public signature of pushCall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 Files:

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep updated this revision to Diff 451134. samestep added a comment. Address Yitzie's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 Files:

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

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

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

ymandel accepted this revision. ymandel added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:4371 +TEST(TransferTest, ContextSensitiveConstructorBody) { + std::string Code = R"(

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

sgatev added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:138 /// + /// `Call` must be either a `CallExpr` or a `CXXConstructExpr`. + /// How about we define overloads that take these types instead of taking

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

ymandel added a comment. Looks good overall. I want to think a bit more about potential additional tests... Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:232 +// This case is disallowed by the precondition from the method docstring. +

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep updated this revision to Diff 450951. samestep added a comment. Fix bug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 Files: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep updated this revision to Diff 450944. samestep added a comment. Tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438 Files: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

samestep created this revision. Herald added subscribers: martong, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. samestep requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo