[PATCH] D150352: [clang][dataflow] Don't analyze templated declarations.

Fri May 12 04:43:47 PDT 2023

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