[NFC][mlir] Update DataFlowFramework.h to be compatible with clang c++23

[philass]

This change makes DataFlowFramework.h compatible with clang++ and --std=c++23.

Previously clang was checking the templated DataFlowSolver::eraseState body before being instantiated. This resulted in issues with incomplete types, and happened at least with clang++-19.

This is fixed by moving the definition of DataFlowSolver::eraseState after the AnalysisState's full class declaration.

For full context:

• https://discourse.llvm.org/t/what-is-the-status-of-c-23-support-in-mlir/87674/12

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Philip Lassen (philass)

Changes

This change makes it possible to compile DataFlowFramework.h with clang and --std=c++23.

For full context:

• https://discourse.llvm.org/t/what-is-the-status-of-c-23-support-in-mlir/87674/12

Thanks for the suggested fix @fabianmcg!

---

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

1 Files Affected:

• (modified) mlir/include/mlir/Analysis/DataFlowFramework.h (+26-23)

diff --git a/mlir/include/mlir/Analysis/DataFlowFramework.h b/mlir/include/mlir/Analysis/DataFlowFramework.h
index 49862927caff2..470170c26e6cb 100644
--- a/mlir/include/mlir/Analysis/DataFlowFramework.h
+++ b/mlir/include/mlir/Analysis/DataFlowFramework.h
@@ -354,29 +354,7 @@ class DataFlowSolver {
 
   /// Erase any analysis state associated with the given lattice anchor.
   template <typename AnchorT>
-  void eraseState(AnchorT anchor) {
-    LatticeAnchor latticeAnchor(anchor);
-
-    // Update equivalentAnchorMap.
-    for (auto &&[TypeId, eqClass] : equivalentAnchorMap) {
-      if (!eqClass.contains(latticeAnchor)) {
-        continue;
-      }
-      llvm::EquivalenceClasses<LatticeAnchor>::member_iterator leaderIt =
-          eqClass.findLeader(latticeAnchor);
-
-      // Update analysis states with new leader if needed.
-      if (*leaderIt == latticeAnchor && ++leaderIt != eqClass.member_end()) {
-        analysisStates[*leaderIt][TypeId] =
-            std::move(analysisStates[latticeAnchor][TypeId]);
-      }
-
-      eqClass.erase(latticeAnchor);
-    }
-
-    // Update analysis states.
-    analysisStates.erase(latticeAnchor);
-  }
+  void eraseState(AnchorT anchor);
 
   /// Erase all analysis states.
   void eraseAllStates() {
@@ -560,6 +538,31 @@ class AnalysisState {
   friend class DataFlowSolver;
 };
 
+template <typename AnchorT>
+void DataFlowSolver::eraseState(AnchorT anchor) {
+  LatticeAnchor latticeAnchor(anchor);
+
+  // Update equivalentAnchorMap.
+  for (auto &&[TypeId, eqClass] : equivalentAnchorMap) {
+    if (!eqClass.contains(latticeAnchor)) {
+      continue;
+    }
+    llvm::EquivalenceClasses<LatticeAnchor>::member_iterator leaderIt =
+        eqClass.findLeader(latticeAnchor);
+
+    // Update analysis states with new leader if needed.
+    if (*leaderIt == latticeAnchor && ++leaderIt != eqClass.member_end()) {
+      analysisStates[*leaderIt][TypeId] =
+          std::move(analysisStates[latticeAnchor][TypeId]);
+    }
+
+    eqClass.erase(latticeAnchor);
+  }
+
+  // Update analysis states.
+  analysisStates.erase(latticeAnchor);
+}
+
 //===----------------------------------------------------------------------===//
 // DataFlowAnalysis
 //===----------------------------------------------------------------------===//

[fabianmcg]

Thank you for this! In general LGTM, I'll approve as soon as the comment is fixed.

[fabianmcg]

Add a comment around the lines:

Suggested change

	template <typename AnchorT>
	//===----------------------------------------------------------------------===//
	// DataFlowSolver definition
	//===----------------------------------------------------------------------===//
	// This is method is defined outside `DataFlowSolver` and after `AnalysisState` to prevent issues around `AnalysisState` being used before it is defined.
	template <typename AnchorT>

[philass]

Done!

[fabianmcg]

Also, please don't reference people in the commit message, see https://discourse.llvm.org/t/forbidding-username-in-commits/86997

[fabianmcg]

Also, please edit the commit message with a bigger description of the issue and solution to not force people to go to discourse. Thanks!

[philass]

Thanks for pointing me to the commit message rules. I updated with a more detailed description of the problem and the fix. Would you like me to delete the reference to the discourse all together?

[philass]

And if you do approve, could you also commit for me? I don't have llvm commit privileges.

[fabianmcg]

Suggested change

	// This is method is defined outside `DataFlowSolver` and after `AnalysisState`
	// This method is defined outside `DataFlowSolver` and after `AnalysisState`

[philass]

Good catch. Done!