Skip to content
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

Revert "[clang][dataflow] Fix unsupported types always being equal" #129761

Merged
merged 1 commit into from
Mar 4, 2025

Conversation

jvoung
Copy link
Contributor

@jvoung jvoung commented Mar 4, 2025

@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 Mar 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Author: Jan Voung (jvoung)

Changes

Reverts llvm/llvm-project#129502

seeing new crashes around https://github.com/google/crubit/blob/859520eca82d60a169fb85cdbf648c57d0a14a99/nullability/test/smart_pointers_diagnosis.cc#L57

Would like some time to investigate.


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

2 Files Affected:

  • (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+1-11)
  • (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (-35)
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index e17a16a3b75d0..9c54eb16d2224 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -60,9 +60,7 @@ static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS,
   Value *LHSValue = Env.getValue(LHS);
   Value *RHSValue = Env.getValue(RHS);
 
-  // When two unsupported values are compared, both are nullptr. Only supported
-  // values should evaluate to equal.
-  if (LHSValue == RHSValue && LHSValue)
+  if (LHSValue == RHSValue)
     return Env.getBoolLiteralValue(true);
 
   if (auto *LHSBool = dyn_cast_or_null<BoolValue>(LHSValue))
@@ -800,14 +798,6 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
     Env.setValue(*S, Env.getIntLiteralValue(S->getValue()));
   }
 
-  // Untyped nullptr's aren't handled by NullToPointer casts, so they need to be
-  // handled separately.
-  void VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *S) {
-    auto &NullPointerVal =
-        Env.getOrCreateNullPointerValue(S->getType()->getPointeeType());
-    Env.setValue(*S, NullPointerVal);
-  }
-
   void VisitParenExpr(const ParenExpr *S) {
     // The CFG does not contain `ParenExpr` as top-level statements in basic
     // blocks, however manual traversal to sub-expressions may encounter them.
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index f52b73dbbdc57..0f731f4532535 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4974,41 +4974,6 @@ TEST(TransferTest, IntegerLiteralEquality) {
       });
 }
 
-TEST(TransferTest, UnsupportedValueEquality) {
-  std::string Code = R"(
-    // An explicitly unsupported type by the framework.
-    enum class EC {
-      A,
-      B
-    };
-  
-    void target() {
-      EC ec = EC::A;
-
-      bool unsupported_eq_same = (EC::A == EC::A);
-      bool unsupported_eq_other = (EC::A == EC::B);
-      bool unsupported_eq_var = (ec == EC::B);
-
-      (void)0; // [[p]]
-    }
-  )";
-  runDataflow(
-      Code,
-      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
-         ASTContext &ASTCtx) {
-        const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
-
-        // We do not model the values of unsupported types, so this
-        // seemingly-trivial case will not be true either.
-        EXPECT_TRUE(isa<AtomicBoolValue>(
-            getValueForDecl<BoolValue>(ASTCtx, Env, "unsupported_eq_same")));
-        EXPECT_TRUE(isa<AtomicBoolValue>(
-            getValueForDecl<BoolValue>(ASTCtx, Env, "unsupported_eq_other")));
-        EXPECT_TRUE(isa<AtomicBoolValue>(
-            getValueForDecl<BoolValue>(ASTCtx, Env, "unsupported_eq_var")));
-      });
-}
-
 TEST(TransferTest, CorrelatedBranches) {
   std::string Code = R"(
     void target(bool B, bool C) {

@jvoung jvoung merged commit d6301b2 into main Mar 4, 2025
13 of 14 checks passed
@jvoung jvoung deleted the revert-129502-dataflow-unsupported-types branch March 4, 2025 20:48
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