From e9fb11fd5a5ee348ad2a7b00c7733d29e74c1f41 Mon Sep 17 00:00:00 2001
From: Joris Vaillant <joris.vaillant@inria.fr>
Date: Tue, 5 Nov 2024 15:26:49 +0100
Subject: [PATCH] core: Avoid a GCC explicit template instantiation and symbol
 visibility bug

---
 .../pinocchio/algorithm/contact-cholesky.hpp  | 46 ++++++-------------
 .../pinocchio/algorithm/contact-cholesky.hxx  | 35 ++++++++++++++
 2 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/include/pinocchio/algorithm/contact-cholesky.hpp b/include/pinocchio/algorithm/contact-cholesky.hpp
index 8f7c9a467..c54941701 100644
--- a/include/pinocchio/algorithm/contact-cholesky.hpp
+++ b/include/pinocchio/algorithm/contact-cholesky.hpp
@@ -53,7 +53,7 @@ namespace pinocchio
   ///
   template<typename _Scalar, int _Options>
   struct PINOCCHIO_UNSUPPORTED_MESSAGE("The API will change towards more flexibility")
-    PINOCCHIO_DLLAPI ContactCholeskyDecompositionTpl
+    ContactCholeskyDecompositionTpl
   {
     EIGEN_MAKE_ALIGNED_OPERATOR_NEW
 
@@ -104,9 +104,7 @@ namespace pinocchio
     ///
     /// \brief Default constructor
     ///
-    ContactCholeskyDecompositionTpl()
-    {
-    }
+    ContactCholeskyDecompositionTpl() = default;
 
     ///
     /// \brief Constructor from a model.
@@ -463,35 +461,10 @@ namespace pinocchio
     ///@}
 
     template<typename S1, int O1>
-    bool operator==(const ContactCholeskyDecompositionTpl<S1, O1> & other) const
-    {
-      bool is_same = true;
-
-      if (nv != other.nv || num_contacts != other.num_contacts)
-        return false;
-
-      if (
-        D.size() != other.D.size() || Dinv.size() != other.Dinv.size() || U.rows() != other.U.rows()
-        || U.cols() != other.U.cols())
-        return false;
-
-      is_same &= (D == other.D);
-      is_same &= (Dinv == other.Dinv);
-      is_same &= (U == other.U);
-
-      is_same &= (parents_fromRow == other.parents_fromRow);
-      is_same &= (nv_subtree_fromRow == other.nv_subtree_fromRow);
-      is_same &= (last_child == other.last_child);
-      //        is_same &= (rowise_sparsity_pattern == other.rowise_sparsity_pattern);
-
-      return is_same;
-    }
+    bool operator==(const ContactCholeskyDecompositionTpl<S1, O1> & other) const;
 
     template<typename S1, int O1>
-    bool operator!=(const ContactCholeskyDecompositionTpl<S1, O1> & other) const
-    {
-      return !(*this == other);
-    }
+    bool operator!=(const ContactCholeskyDecompositionTpl<S1, O1> & other) const;
     PINOCCHIO_COMPILER_DIAGNOSTIC_POP
 
   protected:
@@ -702,10 +675,17 @@ namespace pinocchio
 
 } // namespace pinocchio
 
-#include "pinocchio/algorithm/contact-cholesky.hxx"
-
+// Because of a GCC bug we should NEVER define a function that use ContactCholeskyDecompositionTpl
+// before doing the explicit template instantiation.
+// If we don't take care, GCC will not accept any visibility attribute when declaring the
+// explicit template instantiation of the ContactCholeskyDecompositionTpl class.
+// The warning message will look like this: type attributes ignored after type is already defined
+// [-Wattributes] A minimal code example is added on the PR
+// (https://github.com/stack-of-tasks/pinocchio/pull/2469)
 #if PINOCCHIO_ENABLE_TEMPLATE_INSTANTIATION
   #include "pinocchio/algorithm/contact-cholesky.txx"
 #endif // PINOCCHIO_ENABLE_TEMPLATE_INSTANTIATION
 
+#include "pinocchio/algorithm/contact-cholesky.hxx"
+
 #endif // ifndef __pinocchio_algorithm_contact_cholesky_hpp__
diff --git a/include/pinocchio/algorithm/contact-cholesky.hxx b/include/pinocchio/algorithm/contact-cholesky.hxx
index d16f36928..ece0b0081 100644
--- a/include/pinocchio/algorithm/contact-cholesky.hxx
+++ b/include/pinocchio/algorithm/contact-cholesky.hxx
@@ -625,6 +625,41 @@ namespace pinocchio
     return res;
   }
 
+  template<typename Scalar, int Options>
+  template<typename S1, int O1>
+  bool ContactCholeskyDecompositionTpl<Scalar, Options>::operator==(
+    const ContactCholeskyDecompositionTpl<S1, O1> & other) const
+  {
+    bool is_same = true;
+
+    if (nv != other.nv || num_contacts != other.num_contacts)
+      return false;
+
+    if (
+      D.size() != other.D.size() || Dinv.size() != other.Dinv.size() || U.rows() != other.U.rows()
+      || U.cols() != other.U.cols())
+      return false;
+
+    is_same &= (D == other.D);
+    is_same &= (Dinv == other.Dinv);
+    is_same &= (U == other.U);
+
+    is_same &= (parents_fromRow == other.parents_fromRow);
+    is_same &= (nv_subtree_fromRow == other.nv_subtree_fromRow);
+    is_same &= (last_child == other.last_child);
+    //        is_same &= (rowise_sparsity_pattern == other.rowise_sparsity_pattern);
+
+    return is_same;
+  }
+
+  template<typename Scalar, int Options>
+  template<typename S1, int O1>
+  bool ContactCholeskyDecompositionTpl<Scalar, Options>::operator!=(
+    const ContactCholeskyDecompositionTpl<S1, O1> & other) const
+  {
+    return !(*this == other);
+  }
+
   namespace details
   {