-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang][dataflow] reland #96766 with fix #98896
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
Conversation
… can be evaluated." (llvm#96766) This reverts commit 6e96e5a.
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-analysis Author: None (martinboehme) Changes
Full diff: https://github.com/llvm/llvm-project/pull/98896.diff 3 Files Affected:
diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
index 925b99af9141a..f9c923a36ad22 100644
--- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h
@@ -113,7 +113,11 @@ 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 *) { return true; }
+ bool TraverseCXXTypeidExpr(CXXTypeidExpr *TIE) {
+ if (TIE->isPotentiallyEvaluated())
+ return RecursiveASTVisitor<Derived>::TraverseCXXTypeidExpr(TIE);
+ return true;
+ }
bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *) {
return true;
}
diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
index 5c4d42c6ccdcf..622de6323ad33 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -176,13 +176,18 @@ llvm::Error test::checkDataflowWithNoopAnalysis(
DataflowAnalysisOptions Options, LangStandard::Kind Std,
std::function<llvm::StringMap<QualType>(QualType)> SyntheticFieldCallback) {
llvm::SmallVector<std::string, 3> ASTBuildArgs = {
+ "-fsyntax-only",
// -fnodelayed-template-parsing is the default everywhere but on Windows.
// Set it explicitly so that tests behave the same on Windows as on other
// platforms.
+ "-fno-delayed-template-parsing",
// Set -Wno-unused-value because it's often desirable in tests to write
// expressions with unused value, and we don't want the output to be
// cluttered with warnings about them.
- "-fsyntax-only", "-fno-delayed-template-parsing", "-Wno-unused-value",
+ "-Wno-unused-value",
+ // Some build environments don't have RTTI enabled by default.
+ // Enable it explicitly to make sure tests work in all environments.
+ "-frtti",
"-std=" +
std::string(LangStandard::getLangStandardForKind(Std).getName())};
AnalysisInputs<NoopAnalysis> AI(
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index e743eefa5d458..39e7001393e5e 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1637,6 +1637,49 @@ 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 {
|
Per discussion with @sam-mccall, the issue that was causing the buildbot failure when I originally landed #98896 is that RTTI is turned off by default on Playstation targets. To resolve this, I'm now turning on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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
AnalysisASTVisitor
thattypeid()
can be evaluated." (Revert "[clang][dataflow] TeachAnalysisASTVisitor
thattypeid()
can be evaluated." #96766)checkDataflowWithNoopAnalysis()
.