Skip to content

Revert "[clang][dataflow] Teach AnalysisASTVisitor that typeid() can be evaluated." #96766

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
Jun 26, 2024

Conversation

martinboehme
Copy link
Contributor

Reverts #96731

It causes CI failures.

@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 Jun 26, 2024
@martinboehme martinboehme merged commit 6e96e5a into main Jun 26, 2024
7 of 10 checks passed
@martinboehme martinboehme deleted the revert-96731-piper_export_cl_646718105 branch June 26, 2024 13:40
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2024

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: None (martinboehme)

Changes

Reverts llvm/llvm-project#96731

It causes CI failures.


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

2 Files Affected:

  • (modified) clang/include/clang/Analysis/FlowSensitive/ASTOps.h (+1-5)
  • (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (-43)
diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
index f9c923a36ad22..925b99af9141a 100644
--- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
@@ -113,11 +113,7 @@ class AnalysisASTVisitor : public RecursiveASTVisitor<Derived> {
   // nevertheless it appears in the Clang CFG, so we don't exclude it here.
   bool TraverseDecltypeTypeLoc(DecltypeTypeLoc) { return true; }
   bool TraverseTypeOfExprTypeLoc(TypeOfExprTypeLoc) { return true; }
-  bool TraverseCXXTypeidExpr(CXXTypeidExpr *TIE) {
-    if (TIE->isPotentiallyEvaluated())
-      return RecursiveASTVisitor<Derived>::TraverseCXXTypeidExpr(TIE);
-    return true;
-  }
+  bool TraverseCXXTypeidExpr(CXXTypeidExpr *) { return true; }
   bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *) {
     return true;
   }
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 39e7001393e5e..e743eefa5d458 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1637,49 +1637,6 @@ TEST(TransferTest, StructModeledFieldsWithAccessor) {
       });
 }
 
-TEST(TransferTest, StructModeledFieldsInTypeid) {
-  // Test that we model fields mentioned inside a `typeid()` expression only if
-  // that expression is potentially evaluated -- i.e. if the expression inside
-  // `typeid()` is a glvalue of polymorphic type (see
-  // `CXXTypeidExpr::isPotentiallyEvaluated()` and [expr.typeid]p3).
-  std::string Code = R"(
-    // Definitions needed for `typeid`.
-    namespace std {
-      class type_info {};
-      class bad_typeid {};
-    }  // namespace std
-
-    struct NonPolymorphic {};
-
-    struct Polymorphic {
-      virtual ~Polymorphic() = default;
-    };
-
-    struct S {
-      NonPolymorphic *NonPoly;
-      Polymorphic *Poly;
-    };
-
-    void target(S &s) {
-      typeid(*s.NonPoly);
-      typeid(*s.Poly);
-      // [[p]]
-    }
-  )";
-  runDataflow(
-      Code,
-      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
-         ASTContext &ASTCtx) {
-        const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
-        auto &SLoc = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "s");
-        std::vector<const ValueDecl *> Fields;
-        for (auto [Field, _] : SLoc.children())
-          Fields.push_back(Field);
-        EXPECT_THAT(Fields,
-                    UnorderedElementsAre(findValueDecl(ASTCtx, "Poly")));
-      });
-}
-
 TEST(TransferTest, StructModeledFieldsWithComplicatedInheritance) {
   std::string Code = R"(
     struct Base1 {

arsenm pushed a commit that referenced this pull request Jun 27, 2024
…can be evaluated." (#96766)

Reverts #96731

It causes CI failures.
arsenm pushed a commit that referenced this pull request Jun 27, 2024
…can be evaluated." (#96766)

Reverts #96731

It causes CI failures.
arsenm pushed a commit that referenced this pull request Jun 28, 2024
…can be evaluated." (#96766)

Reverts #96731

It causes CI failures.
arsenm pushed a commit that referenced this pull request Jul 2, 2024
…can be evaluated." (#96766)

Reverts #96731

It causes CI failures.
arsenm pushed a commit that referenced this pull request Jul 3, 2024
…can be evaluated." (#96766)

Reverts #96731

It causes CI failures.
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
arsenm pushed a commit that referenced this pull request Jul 3, 2024
…can be evaluated." (#96766)

Reverts #96731

It causes CI failures.
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
martinboehme added a commit that referenced this pull request Jul 16, 2024
- **Reapply "[clang][dataflow] Teach `AnalysisASTVisitor` that
`typeid()` can be evaluated." (#96766)**
- **Turn on RTTI explicitly in `checkDataflowWithNoopAnalysis()`.**
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
- **Reapply "[clang][dataflow] Teach `AnalysisASTVisitor` that
`typeid()` can be evaluated." (#96766)**
- **Turn on RTTI explicitly in `checkDataflowWithNoopAnalysis()`.**

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251696
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