mboehme marked an inline comment as done.
mboehme added inline comments.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:36
+ /// `S` resides. `D.isTemplated()` must be false.
+ static llvm::Expected<ControlFlowContext> build(const Decl &D, Stmt &S,
+ ASTContext &C);
----------------
gribozavr2 wrote:
> Maybe this is too much for a drive-by fix, but since you are changing this
> API anyway, I have to ask - now that D is nonnull, isn't S strictly
> redundant? Isn't D always a FunctionDecl, and S its body?
> Isn't D always a FunctionDecl, and S its body?
This is true for all of the callsites that I've seen, but I think I remember
talking to someone who said the intent was that you might also use this to
analyze the initializer of a global variable (where `D` would be the variable
declaration and `S` would be the initializer). It seems telling, at least, that
the interface expects only a `Decl`, not a `FunctionDecl`.
So I'm hesitant to remove this API entirely without more investigation. It
certainly makes sense though to add an overload that takes a `FunctionDecl` and
no `Stmt`, which will cover the vast majority of use cases. I'd like to do that
in a separate patch though.
================
Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2537
-TEST(TransferTest, DerefDependentPtr) {
std::string Code = R"(
----------------
gribozavr2 wrote:
> This `DerefDependentPtr` test was originally added in
> https://reviews.llvm.org/D117567 and re-landed in
> https://github.com/llvm/llvm-project/commit/acd4b0359097dd8ad166d569a4566879e82a2793
>
> Could you try to revert the effects of those commits? Specifically:
>
> - remove reference skipping from the transfer function for `UO_Deref` (and
> the comment),
>
> - remove the `-fno-delayed-template-parsing` flag from
> `clang/unittests/Analysis/FlowSensitive/TransferTest.cpp`.
Thanks for pointing this out! Done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150352/new/
https://reviews.llvm.org/D150352
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
