From a77f26ffe36eb20392ff7f7ba7ece3a35f082d3e Mon Sep 17 00:00:00 2001
From: rbock <rbock@eudoxos.de>
Date: Thu, 7 Apr 2016 21:47:17 +0200
Subject: [PATCH] Prevent self-comparison, added more tests, fixed some
 expression checks

---
 include/sqlpp11/basic_expression_operators.h | 78 ++++++++------------
 include/sqlpp11/expression.h                 |  4 +-
 include/sqlpp11/expression_return_types.h    | 14 ++--
 include/sqlpp11/operand_check.h              |  2 +-
 test_static_asserts/AssertTables.h           | 23 ++++++
 test_static_asserts/CMakeLists.txt           |  2 +-
 test_static_asserts/no_self_compare.cpp      | 19 ++++-
 test_static_asserts/text.cpp                 | 28 ++++++-
 8 files changed, 105 insertions(+), 65 deletions(-)

diff --git a/include/sqlpp11/basic_expression_operators.h b/include/sqlpp11/basic_expression_operators.h
index 665de9fb..8c4ae0dd 100644
--- a/include/sqlpp11/basic_expression_operators.h
+++ b/include/sqlpp11/basic_expression_operators.h
@@ -44,38 +44,20 @@ namespace sqlpp
   SQLPP_PORTABLE_STATIC_ASSERT(assert_comparison_valid_rhs_operand_t, "invalid rhs operand in comparison");
   SQLPP_PORTABLE_STATIC_ASSERT(assert_comparison_lhs_rhs_differ_t, "identical lhs and rhs operand types in comparison");
 
-  template <typename LhsType, typename LhsValueType, typename RhsType>
+  template <typename LhsType, typename RhsType>
   using check_rhs_comparison_operand_t = static_combined_check_t<
       static_check_t<(is_expression_t<sqlpp::wrap_operand_t<RhsType>>::value  // expressions are OK
                       or
                       is_multi_expression_t<sqlpp::wrap_operand_t<RhsType>>::value)  // multi-expressions like ANY are
                                                                                      // OK for comparisons, too
                          and
-                         LhsValueType::template _is_valid_operand<
+                         value_type_of<LhsType>::template _is_valid_operand<
                              sqlpp::wrap_operand_t<RhsType>>::value,  // the correct value type is required, of course
                      assert_comparison_valid_rhs_operand_t>,
       static_check_t<not std::is_same<LhsType, RhsType>::value, assert_comparison_lhs_rhs_differ_t>>;
 
-  template <typename LhsType, typename LhsValueType, typename... InTypes>
-  using check_rhs_in_arguments_t =
-      static_combined_check_t<check_rhs_comparison_operand_t<LhsType, LhsValueType, InTypes>...>;
-
-  namespace detail
-  {
-    template <bool Enable, template <typename Lhs> class Expr, typename Lhs>
-    struct new_unary_expression_impl
-    {
-      using type = bad_statement;
-    };
-
-    template <template <typename Lhs> class Expr, typename Lhs>
-    struct new_unary_expression_impl<true, Expr, Lhs>
-    {
-      using type = Expr<Lhs>;
-    };
-  }
-  template <typename Check, template <typename Lhs> class Expr, typename Lhs>
-  using new_unary_expression_t = typename detail::new_unary_expression_impl<Check::value, Expr, Lhs>::type;
+  template <typename LhsType, typename... InTypes>
+  using check_rhs_in_arguments_t = static_combined_check_t<check_rhs_comparison_operand_t<LhsType, InTypes>...>;
 
   namespace detail
   {
@@ -116,53 +98,57 @@ namespace sqlpp
   struct basic_expression_operators
   {
     template <template <typename Lhs, typename Rhs> class NewExpr, typename T>
-    using _new_binary_expression_t =
-        new_binary_expression_t<check_rhs_comparison_operand_t<Expr, ValueType, wrap_operand_t<T>>,
-                                NewExpr,
-                                Expr,
-                                wrap_operand_t<T>>;
+    struct _new_binary_expression
+    {
+      using type = new_binary_expression_t<check_rhs_comparison_operand_t<Expr, wrap_operand_t<T>>,
+                                           NewExpr,
+                                           Expr,
+                                           wrap_operand_t<T>>;
+    };
+    template <template <typename Lhs, typename Rhs> class NewExpr, typename T>
+    using _new_binary_expression_t = typename _new_binary_expression<NewExpr, T>::type;
 
     // workaround for msvs bug
     //	template <template <typename Lhs, typename... Rhs> class NewExpr, typename... T>
     //    using _new_nary_expression_t =
-    //        new_nary_expression_t<logic::all_t<check_rhs_comparison_operand_t<ValueType,
-    //        wrap_operand_t<T>>::value...>,
+    //        new_nary_expression_t<logic::all_t<check_rhs_comparison_operand_t<
+    //                              wrap_operand_t<T>>::value...>,
     //                              NewExpr,
     //                              Expr,
     //                              wrap_operand_t<T>...>;
     template <template <typename Lhs, typename... Rhs> class NewExpr, typename... T>
     struct _new_nary_expression
     {
-      using type = new_nary_expression_t<
-          logic::all_t<check_rhs_comparison_operand_t<Expr, ValueType, wrap_operand_t<T>>::value...>,
-          NewExpr,
-          Expr,
-          wrap_operand_t<T>...>;
+      using type =
+          new_nary_expression_t<logic::all_t<check_rhs_comparison_operand_t<Expr, wrap_operand_t<T>>::value...>,
+                                NewExpr,
+                                Expr,
+                                wrap_operand_t<T>...>;
     };
 
     template <typename T>
     _new_binary_expression_t<equal_to_t, T> operator==(T t) const
     {
       using rhs = wrap_operand_t<T>;
-      check_rhs_comparison_operand_t<Expr, ValueType, rhs>::_();
+      check_rhs_comparison_operand_t<Expr, rhs>::_();
 
-      return {*static_cast<const Expr*>(this), {rhs{t}}};
+      return {*static_cast<const Expr*>(this), rhs{t}};
     }
 
     template <typename T>
     _new_binary_expression_t<not_equal_to_t, T> operator!=(T t) const
     {
       using rhs = wrap_operand_t<T>;
-      check_rhs_comparison_operand_t<Expr, ValueType, rhs>::_();
+      check_rhs_comparison_operand_t<Expr, rhs>::_();
 
-      return {*static_cast<const Expr*>(this), {rhs{t}}};
+      return {*static_cast<const Expr*>(this), rhs{t}};
     }
 
     template <typename T>
     _new_binary_expression_t<less_than_t, T> operator<(T t) const
     {
       using rhs = wrap_operand_t<T>;
-      check_rhs_comparison_operand_t<Expr, ValueType, rhs>::_();
+      check_rhs_comparison_operand_t<Expr, rhs>::_();
 
       return {*static_cast<const Expr*>(this), rhs{t}};
     }
@@ -171,7 +157,7 @@ namespace sqlpp
     _new_binary_expression_t<less_equal_t, T> operator<=(T t) const
     {
       using rhs = wrap_operand_t<T>;
-      check_rhs_comparison_operand_t<Expr, ValueType, rhs>::_();
+      check_rhs_comparison_operand_t<Expr, rhs>::_();
 
       return {*static_cast<const Expr*>(this), rhs{t}};
     }
@@ -180,7 +166,7 @@ namespace sqlpp
     _new_binary_expression_t<greater_than_t, T> operator>(T t) const
     {
       using rhs = wrap_operand_t<T>;
-      check_rhs_comparison_operand_t<Expr, ValueType, rhs>::_();
+      check_rhs_comparison_operand_t<Expr, rhs>::_();
 
       return {*static_cast<const Expr*>(this), rhs{t}};
     }
@@ -189,7 +175,7 @@ namespace sqlpp
     _new_binary_expression_t<greater_equal_t, T> operator>=(T t) const
     {
       using rhs = wrap_operand_t<T>;
-      check_rhs_comparison_operand_t<Expr, ValueType, rhs>::_();
+      check_rhs_comparison_operand_t<Expr, rhs>::_();
 
       return {*static_cast<const Expr*>(this), rhs{t}};
     }
@@ -219,13 +205,13 @@ namespace sqlpp
     //	template <typename... T>
     //	_new_nary_expression_t<in_t, T...> in(T... t) const
     //	{
-    //		check_rhs_in_arguments_t<Expr, ValueType, wrap_operand_t<T>...>::_();
+    //		check_rhs_in_arguments_t<Expr, wrap_operand_t<T>...>::_();
     //		return {*static_cast<const Expr*>(this), wrap_operand_t<T>{t}...};
     //	}
     template <typename... T>
     typename _new_nary_expression<in_t, T...>::type in(T... t) const
     {
-      check_rhs_in_arguments_t<Expr, ValueType, wrap_operand_t<T>...>::_();
+      check_rhs_in_arguments_t<Expr, wrap_operand_t<T>...>::_();
       return {*static_cast<const Expr*>(this), typename wrap_operand<T>::type{t}...};
     }
 
@@ -233,13 +219,13 @@ namespace sqlpp
     //	template <typename... T>
     //	_new_nary_expression_t<not_in_t, T...> not_in(T... t) const
     //	{
-    //		check_rhs_in_arguments_t<Expr, ValueType, wrap_operand_t<T>...>::_();
+    //		check_rhs_in_arguments_t<Expr, wrap_operand_t<T>...>::_();
     //		return {*static_cast<const Expr*>(this), wrap_operand_t<T>{t}...};
     //	}
     template <typename... T>
     typename _new_nary_expression<not_in_t, T...>::type not_in(T... t) const
     {
-      check_rhs_in_arguments_t<Expr, ValueType, wrap_operand_t<T>...>::_();
+      check_rhs_in_arguments_t<Expr, wrap_operand_t<T>...>::_();
       return {*static_cast<const Expr*>(this), typename wrap_operand<T>::type{t}...};
     }
 
diff --git a/include/sqlpp11/expression.h b/include/sqlpp11/expression.h
index a1a611a4..566f8c17 100644
--- a/include/sqlpp11/expression.h
+++ b/include/sqlpp11/expression.h
@@ -48,7 +48,7 @@ namespace sqlpp
     using _rhs_t = rhs_wrap_t<allow_tvin_t<Rhs>, trivial_value_is_null_t<_lhs_t>::value>;
     using _nodes = detail::type_vector<_lhs_t, _rhs_t>;
 
-    binary_expression_t(_lhs_t lhs, _rhs_t rhs) : _lhs(lhs), _rhs(rhs)
+    binary_expression_t(Lhs lhs, Rhs rhs) : _lhs(lhs), _rhs(rhs)
     {
     }
 
@@ -96,7 +96,7 @@ namespace sqlpp
     using _rhs_t = rhs_wrap_t<allow_tvin_t<Rhs>, trivial_value_is_null_t<_lhs_t>::value>;
     using _nodes = detail::type_vector<_lhs_t, _rhs_t>;
 
-    binary_expression_t(Lhs lhs, _rhs_t rhs) : _lhs(lhs), _rhs(rhs)
+    binary_expression_t(Lhs lhs, Rhs rhs) : _lhs(lhs), _rhs(rhs)
     {
     }
 
diff --git a/include/sqlpp11/expression_return_types.h b/include/sqlpp11/expression_return_types.h
index 589a2839..bbec6f6f 100644
--- a/include/sqlpp11/expression_return_types.h
+++ b/include/sqlpp11/expression_return_types.h
@@ -80,7 +80,7 @@ namespace sqlpp
   struct return_type_plus
   {
     using check = assert_valid_operands;
-    using type = bad_expression<boolean>;
+    using type = bad_expression<value_type_of<L>>;
   };
   template <typename L, typename R>
   using return_type_plus_t = typename return_type_plus<L, R>::type;
@@ -89,7 +89,7 @@ namespace sqlpp
   struct return_type_minus
   {
     using check = assert_valid_operands;
-    using type = bad_expression<boolean>;
+    using type = bad_expression<value_type_of<L>>;
   };
   template <typename L, typename R>
   using return_type_minus_t = typename return_type_minus<L, R>::type;
@@ -98,7 +98,7 @@ namespace sqlpp
   struct return_type_multiplies
   {
     using check = assert_valid_operands;
-    using type = bad_expression<boolean>;
+    using type = bad_expression<value_type_of<L>>;
   };
   template <typename L, typename R>
   using return_type_multiplies_t = typename return_type_multiplies<L, R>::type;
@@ -107,7 +107,7 @@ namespace sqlpp
   struct return_type_divides
   {
     using check = assert_valid_operands;
-    using type = bad_expression<boolean>;
+    using type = bad_expression<value_type_of<L>>;
   };
   template <typename L, typename R>
   using return_type_divides_t = typename return_type_divides<L, R>::type;
@@ -116,7 +116,7 @@ namespace sqlpp
   struct return_type_modulus
   {
     using check = assert_valid_operands;
-    using type = bad_expression<boolean>;
+    using type = bad_expression<value_type_of<L>>;
   };
   template <typename L, typename R>
   using return_type_modulus_t = typename return_type_modulus<L, R>::type;
@@ -125,7 +125,7 @@ namespace sqlpp
   struct return_type_unary_plus
   {
     using check = assert_valid_operands;
-    using type = bad_expression<boolean>;
+    using type = bad_expression<value_type_of<T>>;
   };
   template <typename T, typename Defer>
   using return_type_unary_plus_t = typename return_type_unary_plus<T, Defer>::type;
@@ -134,7 +134,7 @@ namespace sqlpp
   struct return_type_unary_minus
   {
     using check = assert_valid_operands;
-    using type = bad_expression<boolean>;
+    using type = bad_expression<value_type_of<T>>;
   };
   template <typename T, typename Defer>
   using return_type_unary_minus_t = typename return_type_unary_minus<T, Defer>::type;
diff --git a/include/sqlpp11/operand_check.h b/include/sqlpp11/operand_check.h
index 1de5abec..4060d7dc 100644
--- a/include/sqlpp11/operand_check.h
+++ b/include/sqlpp11/operand_check.h
@@ -66,7 +66,7 @@ namespace sqlpp
 
   template <typename L, template <typename> class LPred, typename R, template <typename> class RPred>
   using binary_operand_check_t =
-      typename binary_operand_check<wrap_operand_t<L>, LPred, wrap_operand_t<L>, RPred>::type;
+      typename binary_operand_check<wrap_operand_t<L>, LPred, wrap_operand_t<R>, RPred>::type;
 
   template <typename L, template <typename> class LPred, typename R, template <typename> class RPred>
   using unwrapped_binary_operand_check_t = typename binary_operand_check<L, LPred, R, RPred>::type;
diff --git a/test_static_asserts/AssertTables.h b/test_static_asserts/AssertTables.h
index a8ea1f8d..d21dae53 100644
--- a/test_static_asserts/AssertTables.h
+++ b/test_static_asserts/AssertTables.h
@@ -142,6 +142,28 @@ namespace test
       };
       using _traits = sqlpp::make_traits<sqlpp::time_point, sqlpp::tag::can_be_null>;
     };
+    struct OtherString
+    {
+      struct _alias_t
+      {
+        static constexpr const char _literal[] = "other_string";
+        using _name_t = sqlpp::make_char_sequence<sizeof(_literal), _literal>;
+        template <typename T>
+        struct _member_t
+        {
+          T otherString;
+          T& operator()()
+          {
+            return otherString;
+          }
+          const T& operator()() const
+          {
+            return otherString;
+          }
+        };
+      };
+      using _traits = sqlpp::make_traits<sqlpp::varchar, sqlpp::tag::can_be_null>;
+    };
     struct OtherInt
     {
       struct _alias_t
@@ -261,6 +283,7 @@ namespace test
                                       TabAllTypes_::SomeBool,
                                       TabAllTypes_::SomeDayPoint,
                                       TabAllTypes_::SomeTimePoint,
+                                      TabAllTypes_::OtherString,
                                       TabAllTypes_::OtherInt,
                                       TabAllTypes_::OtherFloat,
                                       TabAllTypes_::OtherBool,
diff --git a/test_static_asserts/CMakeLists.txt b/test_static_asserts/CMakeLists.txt
index 338f3cd9..6726a39d 100644
--- a/test_static_asserts/CMakeLists.txt
+++ b/test_static_asserts/CMakeLists.txt
@@ -23,7 +23,7 @@
 # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 function(test_compile name)
-  set(target sqlpp11_${name})
+  set(target sqlpp11_assert_${name})
   add_executable(${target} ${name}.cpp)
   target_link_libraries(${target} PRIVATE sqlpp11 sqlpp11_testing)
 endfunction()
diff --git a/test_static_asserts/no_self_compare.cpp b/test_static_asserts/no_self_compare.cpp
index 0621897b..3b819a18 100644
--- a/test_static_asserts/no_self_compare.cpp
+++ b/test_static_asserts/no_self_compare.cpp
@@ -44,7 +44,7 @@ namespace
   }
 
   template <typename Assert, typename Operand>
-  void static_check_comparison(const Operand& operand)
+  void static_check_self_compare(const Operand& operand)
   {
     using CheckResult = sqlpp::check_rhs_comparison_operand_t<Operand, Operand>;
     using ExpectedCheckResult = std::is_same<CheckResult, Assert>;
@@ -63,12 +63,23 @@ namespace
 
   void disallowed_self_comparison()
   {
-    // static_check_comparison<sqlpp::assert_comparison_valid_rhs_operand_t>(t.someString);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.someString);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.someInt);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.someFloat);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.someBool);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.someDayPoint);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.someTimePoint);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.otherString);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.otherInt);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.otherFloat);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.otherBool);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.otherDayPoint);
+    static_check_self_compare<sqlpp::assert_comparison_lhs_rhs_differ_t>(t.otherTimePoint);
   }
 }
 
 int main(int, char* [])
 {
-  t.someString == t.someString;
-  // disallowed_self_comparison();
+  // t.someString == t.someString;
+  disallowed_self_comparison();
 }
diff --git a/test_static_asserts/text.cpp b/test_static_asserts/text.cpp
index 0cdb350a..eb9cd94d 100644
--- a/test_static_asserts/text.cpp
+++ b/test_static_asserts/text.cpp
@@ -66,9 +66,9 @@ namespace
   void allowed_comparands()
   {
     static_check_comparison<sqlpp::consistent_t>("");
-    static_check_comparison<sqlpp::consistent_t>('d');
+    // static_check_comparison<sqlpp::consistent_t>('d'); // not today
     static_check_comparison<sqlpp::consistent_t>(std::string(""));
-    static_check_comparison<sqlpp::consistent_t>(t.otherText);
+    static_check_comparison<sqlpp::consistent_t>(t.otherString);
   }
 
   void disallowed_comparands()
@@ -82,12 +82,32 @@ namespace
     static_check_comparison<sqlpp::assert_comparison_valid_rhs_operand_t>(t.someDayPoint);
     static_check_comparison<sqlpp::assert_comparison_valid_rhs_operand_t>(t.someTimePoint);
   }
+
+  template <typename Expected, typename Expression>
+  void static_check_type()
+  {
+    static_assert(std::is_same<Expected, Expression>::value, "Unexpected check result");
+  }
+
+  auto check_expressions() -> void
+  {
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString + 1)>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString - 1)>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString - "")>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString * 1)>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString * "")>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString / 1)>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString / "")>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString % 1)>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(t.someString % "")>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(-t.someString)>();
+    static_check_type<sqlpp::bad_expression<sqlpp::text>, decltype(+t.someString)>();
+  }
 }
 
 int main(int, char* [])
 {
-#error : This must fail:
-  t.someString + 1;
   allowed_comparands();
   disallowed_comparands();
+  check_expressions();
 }