[clang] [clang][dataflow] Avoid putting an assertion in an `LLVM_DEBUG` block. (PR #67313)

via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 25 04:18:50 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-analysis

<details>
<summary>Changes</summary>

`LLVM_DEBUG` blocks are only run if the `-debug` command line flag is passed.
We don't do this in any of our CI builds, so the assertion has limited value and
it's likely it will start failing over time.


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


1 Files Affected:

- (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+16-12) 


``````````diff
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 2414a1cc026af5f..2ad3f9d7c94a80c 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -696,19 +696,23 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
       FieldLocs.insert({Field, &Loc});
     }
 
-    LLVM_DEBUG({
-      // Check that we satisfy the invariant that a `RecordStorageLoation`
-      // contains exactly the set of modeled fields for that type.
-      // `ModeledFields` includes fields from all the bases, but only the
-      // modeled ones. However, if a class type is initialized with an
-      // `InitListExpr`, all fields in the class, including those from base
-      // classes, are included in the set of modeled fields. The code above
-      // should therefore populate exactly the modeled fields.
-      auto ModeledFields = Env.getDataflowAnalysisContext().getModeledFields(Type);
-      assert(ModeledFields.size() == FieldLocs.size());
+    // Check that we satisfy the invariant that a `RecordStorageLoation`
+    // contains exactly the set of modeled fields for that type.
+    // `ModeledFields` includes fields from all the bases, but only the
+    // modeled ones. However, if a class type is initialized with an
+    // `InitListExpr`, all fields in the class, including those from base
+    // classes, are included in the set of modeled fields. The code above
+    // should therefore populate exactly the modeled fields.
+    assert([&]() {
+      auto ModeledFields =
+          Env.getDataflowAnalysisContext().getModeledFields(Type);
+      if (ModeledFields.size() != FieldLocs.size())
+        return false;
       for ([[maybe_unused]] auto [Field, Loc] : FieldLocs)
-        assert(ModeledFields.contains(cast_or_null<FieldDecl>(Field)));
-    });
+        if (!ModeledFields.contains(cast_or_null<FieldDecl>(Field)))
+          return false;
+      return true;
+    }());
 
     auto &Loc =
         Env.getDataflowAnalysisContext().arena().create<RecordStorageLocation>(

``````````

</details>


https://github.com/llvm/llvm-project/pull/67313


More information about the cfe-commits mailing list