Big 853: replace enable_if in Ref<> ctor by static assertions and add failtests for Ref<>

9 files changed, 100 insertions, 11 deletions

diff --git a/Eigen/src/Core/Ref.h b/Eigen/src/Core/Ref.h
index 27fa178cd..bc7ce9a14 100644
--- a/Eigen/src/Core/Ref.h
+++ b/Eigen/src/Core/Ref.h
@@ -184,6 +184,8 @@ template<typename PlainObjectType, int Options, typename StrideType> class Ref
   : public RefBase<Ref<PlainObjectType, Options, StrideType> >
 {
     typedef internal::traits<Ref> Traits;
+    template<typename Derived>
+    EIGEN_DEVICE_FUNC inline Ref(const PlainObjectBase<Derived>& expr);
   public:
 
     typedef RefBase<Ref> Base;
@@ -192,20 +194,21 @@ template<typename PlainObjectType, int Options, typename StrideType> class Ref
 
     #ifndef EIGEN_PARSED_BY_DOXYGEN
     template<typename Derived>
-    EIGEN_DEVICE_FUNC inline Ref(PlainObjectBase<Derived>& expr,
-               typename internal::enable_if<bool(Traits::template match<Derived>::MatchAtCompileTime),Derived>::type* = 0)
+    EIGEN_DEVICE_FUNC inline Ref(PlainObjectBase<Derived>& expr)
     {
-      Base::construct(expr);
+      EIGEN_STATIC_ASSERT(bool(Traits::template match<Derived>::MatchAtCompileTime), STORAGE_LAYOUT_DOES_NOT_MATCH);
+      Base::construct(expr.derived());
     }
     template<typename Derived>
-    EIGEN_DEVICE_FUNC inline Ref(const DenseBase<Derived>& expr,
-               typename internal::enable_if<bool(internal::is_lvalue<Derived>::value&&bool(Traits::template match<Derived>::MatchAtCompileTime)),Derived>::type* = 0,
-               int = Derived::ThisConstantIsPrivateInPlainObjectBase)
+    EIGEN_DEVICE_FUNC inline Ref(const DenseBase<Derived>& expr)
     #else
     template<typename Derived>
     inline Ref(DenseBase<Derived>& expr)
     #endif
     {
+      EIGEN_STATIC_ASSERT(bool(internal::is_lvalue<Derived>::value), THIS_EXPRESSION_IS_NOT_A_LVALUE__IT_IS_READ_ONLY);
+      EIGEN_STATIC_ASSERT(bool(Traits::template match<Derived>::MatchAtCompileTime), STORAGE_LAYOUT_DOES_NOT_MATCH);
+      enum { THIS_EXPRESSION_IS_NOT_A_LVALUE__IT_IS_READ_ONLY = Derived::ThisConstantIsPrivateInPlainObjectBase};
       Base::construct(expr.const_cast_derived());
     }
 
diff --git a/Eigen/src/Core/util/StaticAssert.h b/Eigen/src/Core/util/StaticAssert.h
index f53dd9bfe..7538a0633 100644
--- a/Eigen/src/Core/util/StaticAssert.h
+++ b/Eigen/src/Core/util/StaticAssert.h
@@ -92,7 +92,8 @@
         THE_INDEX_TYPE_MUST_BE_A_SIGNED_TYPE,
         THE_STORAGE_ORDER_OF_BOTH_SIDES_MUST_MATCH,
         OBJECT_ALLOCATED_ON_STACK_IS_TOO_BIG,
-        IMPLICIT_CONVERSION_TO_SCALAR_IS_FOR_INNER_PRODUCT_ONLY
+        IMPLICIT_CONVERSION_TO_SCALAR_IS_FOR_INNER_PRODUCT_ONLY,
+        STORAGE_LAYOUT_DOES_NOT_MATCH
       };
     };
 
diff --git a/failtest/CMakeLists.txt b/failtest/CMakeLists.txt
index 83b6c81a9..5343dc152 100644
--- a/failtest/CMakeLists.txt
+++ b/failtest/CMakeLists.txt
@@ -32,6 +32,12 @@ ei_add_failtest("cwiseunaryview_on_const_type_actually_const")
 ei_add_failtest("triangularview_on_const_type_actually_const")
 ei_add_failtest("selfadjointview_on_const_type_actually_const")
 
+ei_add_failtest("ref_1")
+ei_add_failtest("ref_2")
+ei_add_failtest("ref_3")
+ei_add_failtest("ref_4")
+ei_add_failtest("ref_5")
+
 if (EIGEN_FAILTEST_FAILURE_COUNT)
   message(FATAL_ERROR
           "${EIGEN_FAILTEST_FAILURE_COUNT} out of ${EIGEN_FAILTEST_COUNT} failtests FAILED. "
diff --git a/failtest/ref_1.cpp b/failtest/ref_1.cpp
new file mode 100644
index 000000000..8b798d53d
--- /dev/null
+++ b/failtest/ref_1.cpp
@@ -0,0 +1,18 @@
+#include "../Eigen/Core"
+
+#ifdef EIGEN_SHOULD_FAIL_TO_BUILD
+#define CV_QUALIFIER const
+#else
+#define CV_QUALIFIER
+#endif
+
+using namespace Eigen;
+
+void call_ref(Ref<VectorXf> a) { }
+
+int main()
+{
+  VectorXf a(10);
+  CV_QUALIFIER VectorXf& ac(a);
+  call_ref(ac);
+}
diff --git a/failtest/ref_2.cpp b/failtest/ref_2.cpp
new file mode 100644
index 000000000..0b779ccf5
--- /dev/null
+++ b/failtest/ref_2.cpp
@@ -0,0 +1,15 @@
+#include "../Eigen/Core"
+
+using namespace Eigen;
+
+void call_ref(Ref<VectorXf> a) { }
+
+int main()
+{
+  MatrixXf A(10,10);
+#ifdef EIGEN_SHOULD_FAIL_TO_BUILD
+  call_ref(A.row(3));
+#else
+  call_ref(A.col(3));
+#endif
+}
diff --git a/failtest/ref_3.cpp b/failtest/ref_3.cpp
new file mode 100644
index 000000000..f46027d48
--- /dev/null
+++ b/failtest/ref_3.cpp
@@ -0,0 +1,15 @@
+#include "../Eigen/Core"
+
+using namespace Eigen;
+
+#ifdef EIGEN_SHOULD_FAIL_TO_BUILD
+void call_ref(Ref<VectorXf> a) { }
+#else
+void call_ref(const Ref<const VectorXf> &a) { }
+#endif
+
+int main()
+{
+  VectorXf a(10);
+  call_ref(a+a);
+}
diff --git a/failtest/ref_4.cpp b/failtest/ref_4.cpp
new file mode 100644
index 000000000..6c11fa4cb
--- /dev/null
+++ b/failtest/ref_4.cpp
@@ -0,0 +1,15 @@
+#include "../Eigen/Core"
+
+using namespace Eigen;
+
+void call_ref(Ref<MatrixXf,0,OuterStride<> > a) {}
+
+int main()
+{
+  MatrixXf A(10,10);
+#ifdef EIGEN_SHOULD_FAIL_TO_BUILD
+  call_ref(A.transpose());
+#else
+  call_ref(A);
+#endif
+}
diff --git a/failtest/ref_5.cpp b/failtest/ref_5.cpp
new file mode 100644
index 000000000..846d52795
--- /dev/null
+++ b/failtest/ref_5.cpp
@@ -0,0 +1,16 @@
+#include "../Eigen/Core"
+
+using namespace Eigen;
+
+void call_ref(Ref<VectorXf> a) { }
+
+int main()
+{
+  VectorXf a(10);
+  DenseBase<VectorXf> &ac(a);
+#ifdef EIGEN_SHOULD_FAIL_TO_BUILD
+  call_ref(ac);
+#else
+  call_ref(ac.derived());
+#endif
+}
diff --git a/test/ref.cpp b/test/ref.cpp
index d91e3b54c..b9470213c 100644
--- a/test/ref.cpp
+++ b/test/ref.cpp
@@ -182,15 +182,15 @@ void call_ref()
 
   VERIFY_EVALUATION_COUNT( call_ref_1(a,a), 0);
   VERIFY_EVALUATION_COUNT( call_ref_1(b,b.transpose()), 0);
-//   call_ref_1(ac);           // does not compile because ac is const
+//   call_ref_1(ac,a<c);           // does not compile because ac is const
   VERIFY_EVALUATION_COUNT( call_ref_1(ab,ab), 0);
   VERIFY_EVALUATION_COUNT( call_ref_1(a.head(4),a.head(4)), 0);
   VERIFY_EVALUATION_COUNT( call_ref_1(abc,abc), 0);
   VERIFY_EVALUATION_COUNT( call_ref_1(A.col(3),A.col(3)), 0);
-//   call_ref_1(A.row(3));    // does not compile because innerstride!=1
+//   call_ref_1(A.row(3),A.row(3));    // does not compile because innerstride!=1
   VERIFY_EVALUATION_COUNT( call_ref_3(A.row(3),A.row(3).transpose()), 0);
   VERIFY_EVALUATION_COUNT( call_ref_4(A.row(3),A.row(3).transpose()), 0);
-//   call_ref_1(a+a);          // does not compile for obvious reason
+//   call_ref_1(a+a, a+a);          // does not compile for obvious reason
 
   MatrixXf tmp = A*A.col(1);
   VERIFY_EVALUATION_COUNT( call_ref_2(A*A.col(1), tmp), 1);     // evaluated into a temp
@@ -211,7 +211,7 @@ void call_ref()
   VERIFY_EVALUATION_COUNT( call_ref_5(a,a), 0);
   VERIFY_EVALUATION_COUNT( call_ref_5(a.head(3),a.head(3)), 0);
   VERIFY_EVALUATION_COUNT( call_ref_5(A,A), 0);
-//   call_ref_5(A.transpose());   // does not compile
+//   call_ref_5(A.transpose(),A.transpose());   // does not compile because storage order does not match
   VERIFY_EVALUATION_COUNT( call_ref_5(A.block(1,1,2,2),A.block(1,1,2,2)), 0);
   VERIFY_EVALUATION_COUNT( call_ref_5(b,b), 0);             // storage order do not match, but this is a degenerate case that should work
   VERIFY_EVALUATION_COUNT( call_ref_5(a.row(3),a.row(3)), 0);