Skip to content

Revert "[clang][dataflow] Retrieve members from accessors called using member…" #74299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

martinboehme
Copy link
Contributor

Reverts #73978

@martinboehme martinboehme merged commit 3b6d63c into main Dec 4, 2023
@martinboehme martinboehme deleted the revert-73978-piper_export_cl_583843651 branch December 4, 2023 10:27
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang:analysis labels Dec 4, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2023

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: None (martinboehme)

Changes

Reverts llvm/llvm-project#73978


Full diff: https://github.com/llvm/llvm-project/pull/74299.diff

2 Files Affected:

  • (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+2-5)
  • (modified) clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp (-51)
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index b98037b736452..042402a129d10 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,12 +300,9 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
-  // Use getCalleeDecl instead of getMethodDecl in order to handle
-  // pointer-to-member calls.
-  const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(C.getCalleeDecl());
-  if (!MethodDecl)
+  if (!C.getMethodDecl())
     return nullptr;
-  auto *Body = dyn_cast_or_null<CompoundStmt>(MethodDecl->getBody());
+  auto *Body = dyn_cast_or_null<CompoundStmt>(C.getMethodDecl()->getBody());
   if (!Body || Body->size() != 1)
     return nullptr;
   if (auto *RS = dyn_cast<ReturnStmt>(*Body->body_begin()))
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index c0f59c9de4131..3569b0eac7005 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -25,7 +25,6 @@ namespace {
 using namespace clang;
 using namespace dataflow;
 using ::clang::dataflow::test::getFieldValue;
-using ::testing::Contains;
 using ::testing::IsNull;
 using ::testing::NotNull;
 
@@ -312,56 +311,6 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) {
   EXPECT_THAT(Env.getValue(*Var), NotNull());
 }
 
-// Pointers to Members are a tricky case of accessor calls, complicated further
-// when using templates where the pointer to the member is a template argument.
-// This is a repro of a failure case seen in the wild.
-TEST_F(EnvironmentTest,
-       ModelMemberForAccessorUsingMethodPointerThroughTemplate) {
-  using namespace ast_matchers;
-
-  std::string Code = R"cc(
-      struct S {
-        int accessor() {return member;}
-
-        int member = 0;
-      };
-
-      template <auto method>
-      int Target(S* S) {
-        return (S->*method)();
-      }
-
-     // We want to analyze the instantiation of Target for the accessor.
-     int Instantiator () {S S; return Target<&S::accessor>(&S); }
-  )cc";
-
-  auto Unit =
-      // C++17 for the simplifying use of auto in the template declaration.
-      tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
-  auto &Context = Unit->getASTContext();
-
-  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
-
-  auto Results = match(
-      decl(anyOf(functionDecl(hasName("Target"), isTemplateInstantiation())
-                     .bind("target"),
-                 fieldDecl(hasName("member")).bind("member"),
-                 recordDecl(hasName("S")).bind("struct"))),
-      Context);
-  const auto *Fun = selectFirst<FunctionDecl>("target", Results);
-  const auto *Struct = selectFirst<RecordDecl>("struct", Results);
-  const auto *Member = selectFirst<FieldDecl>("member", Results);
-  ASSERT_THAT(Fun, NotNull());
-  ASSERT_THAT(Struct, NotNull());
-  ASSERT_THAT(Member, NotNull());
-
-  // Verify that `member` is modeled for `S` when we analyze
-  // `Target<&S::accessor>`.
-  Environment Env(DAContext, *Fun);
-  EXPECT_THAT(DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 0)),
-              Contains(Member));
-}
-
 TEST_F(EnvironmentTest, RefreshRecordValue) {
   using namespace ast_matchers;
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:analysis clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants