Fix Broken Floating Point Expressions

Fixes https://github.com/DOCGroup/ACE_TAO/issues/1713 and adds a test.

Also makes some other minor fixes and improvments to AST_Expression.

4 files changed, 246 insertions, 89 deletions

diff --git a/TAO/TAO_IDL/ast/ast_expression.cpp b/TAO/TAO_IDL/ast/ast_expression.cpp
index 276f0f66cac..0434ad41671 100644
--- a/TAO/TAO_IDL/ast/ast_expression.cpp
+++ b/TAO/TAO_IDL/ast/ast_expression.cpp
@@ -98,6 +98,7 @@ AST_Expression::eval_kind_to_expr_type (AST_Expression::EvalKind eval_kind)
     case EK_long:
       return EV_long;
     case EK_ulong:
+    case EK_positive_int:
       return EV_ulong;
     case EK_longlong:
       return EV_longlong;
@@ -113,9 +114,13 @@ AST_Expression::eval_kind_to_expr_type (AST_Expression::EvalKind eval_kind)
       return EV_int8;
     case EK_uint8:
       return EV_uint8;
-    default:
+    case EK_const:
+      idl_global->err ()->misc_error ("eval_kind_to_expr_type can't handle EK_const");
       return EV_none;
     }
+
+  idl_global->err ()->misc_error ("eval_kind_to_expr_type unhandled EvalKind");
+  return EV_none;
 }
 
 // Helper function to fill out the details of where this expression
@@ -1650,7 +1655,8 @@ eval_kind (AST_Expression::AST_ExprValue *ev, AST_Expression::EvalKind ek)
     case AST_Expression::EK_uint8:
       retval = coerce_value (newval, AST_Expression::EV_uint8);
       break;
-    default:
+    case AST_Expression::EK_floating_point:
+      retval = coerce_value (newval, AST_Expression::EV_double);
       break;
   }
 
@@ -1736,7 +1742,8 @@ AST_Expression::eval_bin_op (AST_Expression::EvalKind ek)
       return nullptr;
     }
 
-  ExprType const expr_type = eval_kind_to_expr_type (ek);
+  ExprType const expr_type = ek == EK_const ?
+    pd_v1->ev()->et : eval_kind_to_expr_type (ek);
   if (expr_type == EV_none) return nullptr;
 
   ACE_NEW_RETURN (retval,
@@ -1806,7 +1813,7 @@ AST_Expression::eval_bin_op (AST_Expression::EvalKind ek)
       break;
 
     default:
-      success = true;
+      break;
     }
 
   if (!success)
@@ -1818,6 +1825,17 @@ AST_Expression::eval_bin_op (AST_Expression::EvalKind ek)
   return retval;
 }
 
+template <typename Type>
+bool
+do_eval_mod_op (Type a, Type b, Type &result)
+{
+  if (b == 0) {
+    return false;
+  }
+  result = a % b;
+  return true;
+}
+
 // Apply binary operators to an AST_Expression after evaluating
 // its sub-expressions.
 // Operations supported: '%'
@@ -1839,79 +1857,79 @@ AST_Expression::eval_mod_op (AST_Expression::EvalKind ek)
       return nullptr;
     }
 
+  ExprType const expr_type = ek == EK_const ?
+    pd_v1->ev()->et : eval_kind_to_expr_type (ek);
+  if (expr_type == EV_none) return nullptr;
+
   ACE_NEW_RETURN (retval,
                   AST_ExprValue,
                   nullptr);
 
-  if (ek == EK_ulonglong)
-    {
-      this->pd_v1->set_ev (this->pd_v1->coerce (EV_ulonglong));
-      this->pd_v2->set_ev (this->pd_v2->coerce (EV_ulonglong));
-      retval->et = EV_ulonglong;
+  pd_v1->set_ev (pd_v1->coerce (expr_type));
+  pd_v2->set_ev (pd_v2->coerce (expr_type));
+  retval->et = expr_type;
 
-      if (this->pd_v2->ev ()->u.ullval == 0)
-        {
-          delete retval;
-          retval = nullptr;
-          return nullptr;
-        }
+  bool success = false;
+  switch (expr_type)
+  {
+    case EV_int8:
+      success = do_eval_mod_op<ACE_CDR::Int8> (
+        pd_v1->ev ()->u.int8val, pd_v2->ev ()->u.int8val, retval->u.int8val);
+      break;
 
-      retval->u.ullval =
-        this->pd_v1->ev ()->u.ullval % this->pd_v2->ev ()->u.ullval;
-    }
-  else if (ek == EK_longlong)
-    {
-      this->pd_v1->set_ev (this->pd_v1->coerce (EV_longlong));
-      this->pd_v2->set_ev (this->pd_v2->coerce (EV_longlong));
-      retval->et = EV_longlong;
+    case EV_uint8:
+      success = do_eval_mod_op<ACE_CDR::UInt8> (
+        pd_v1->ev ()->u.uint8val, pd_v2->ev ()->u.uint8val, retval->u.uint8val);
+      break;
 
-      if (this->pd_v2->ev ()->u.llval == 0)
-        {
-          delete retval;
-          retval = nullptr;
-          return nullptr;
-        }
+    case EV_short:
+      success = do_eval_mod_op<ACE_CDR::Short> (
+        pd_v1->ev ()->u.sval, pd_v2->ev ()->u.sval, retval->u.sval);
+      break;
 
-      retval->u.llval =
-        this->pd_v1->ev ()->u.llval % this->pd_v2->ev ()->u.llval;
-    }
-  else if (ek == EK_ulong)
-    {
-      this->pd_v1->set_ev (this->pd_v1->coerce (EV_ulong));
-      this->pd_v2->set_ev (this->pd_v2->coerce (EV_ulong));
-      retval->et = EV_ulong;
+    case EV_ushort:
+      success = do_eval_mod_op<ACE_CDR::UShort> (
+        pd_v1->ev ()->u.usval, pd_v2->ev ()->u.usval, retval->u.usval);
+      break;
 
-      if (this->pd_v2->ev ()->u.ulval == 0)
-        {
-          delete retval;
-          retval = nullptr;
-          return nullptr;
-        }
+    case EV_long:
+      success = do_eval_mod_op<ACE_CDR::Long> (
+        pd_v1->ev ()->u.lval, pd_v2->ev ()->u.lval, retval->u.lval);
+      break;
 
-      retval->u.ulval =
-        this->pd_v1->ev ()->u.ulval % this->pd_v2->ev ()->u.ulval;
-    }
-  else if (ek == EK_long)
-    {
-      this->pd_v1->set_ev (this->pd_v1->coerce (EV_long));
-      this->pd_v2->set_ev (this->pd_v2->coerce (EV_long));
-      retval->et = EV_long;
+    case EV_ulong:
+      success = do_eval_mod_op<ACE_CDR::ULong> (
+        pd_v1->ev ()->u.ulval, pd_v2->ev ()->u.ulval, retval->u.ulval);
+      break;
 
-      if (this->pd_v2->ev ()->u.lval == 0)
-        {
-          delete retval;
-          retval = nullptr;
-          return nullptr;
-        }
+    case EV_longlong:
+      success = do_eval_mod_op<ACE_CDR::LongLong> (
+        pd_v1->ev ()->u.llval, pd_v2->ev ()->u.llval, retval->u.llval);
+      break;
 
-      retval->u.lval =
-        this->pd_v1->ev ()->u.lval % this->pd_v2->ev ()->u.lval;
-    }
-  else
+    case EV_ulonglong:
+      success = do_eval_mod_op<ACE_CDR::ULongLong> (
+        pd_v1->ev ()->u.ullval, pd_v2->ev ()->u.ullval, retval->u.ullval);
+      break;
+
+    case EV_octet:
+      success = do_eval_mod_op<ACE_CDR::Octet> (
+        pd_v1->ev ()->u.oval, pd_v2->ev ()->u.oval, retval->u.oval);
+      break;
+
+    case EV_bool:
+      success = do_eval_mod_op<ACE_CDR::Boolean> (
+        pd_v1->ev ()->u.bval, pd_v2->ev ()->u.bval, retval->u.bval);
+      break;
+
+    default:
+      break;
+  }
+
+  if (!success)
     {
       delete retval;
       retval = nullptr;
-      return nullptr;
     }
 
   return retval;
@@ -1991,7 +2009,7 @@ AST_Expression::eval_bit_op (AST_Expression::EvalKind ek)
   pd_v2->set_ev (pd_v2->coerce (expr_type));
   retval->et = expr_type;
 
-  bool success = true;
+  bool success = false;
   switch (expr_type)
   {
     case EV_int8:
@@ -2045,7 +2063,7 @@ AST_Expression::eval_bit_op (AST_Expression::EvalKind ek)
       break;
 
     default:
-      success = true;
+      break;
   }
 
   if (!success)
@@ -2163,10 +2181,10 @@ AST_Expression::eval_un_op (AST_Expression::EvalKind ek)
         case EV_octet:
           retval->u.oval = ~this->pd_v1->ev ()->u.oval;
           break;
-        case AST_Expression::EV_int8:
+        case EV_int8:
           retval->u.int8val = ~pd_v1->ev ()->u.int8val;
           break;
-        case AST_Expression::EV_uint8:
+        case EV_uint8:
           retval->u.uint8val = ~pd_v1->ev ()->u.uint8val;
           break;
         default:
@@ -2372,8 +2390,21 @@ AST_Expression::coerce (AST_Expression::ExprType t)
     case EV_fixed:
       tmp = this->eval_internal (EK_fixed_point);
       break;
-    default:
-      tmp = this->eval_internal (EK_const);
+    case EV_float:
+    case EV_double:
+    case EV_longdouble:
+      tmp = eval_internal (EK_floating_point);
+      break;
+    case EV_char:
+    case EV_wchar:
+    case EV_string:
+    case EV_wstring:
+    case EV_enum:
+    case EV_any:
+    case EV_object:
+    case EV_void:
+      tmp = eval_internal (EK_const);
+    case EV_none:
       break;
   }
 
@@ -2457,7 +2488,8 @@ AST_Expression::coerce (AST_Expression::ExprType t)
     case EV_uint8:
       copy->u.uint8val = this->pd_ev->u.uint8val;
       break;
-    default:
+    case EV_any:
+    case EV_object:
       break;
   }
 
@@ -2677,29 +2709,29 @@ dump_expr_val (ACE_OSTREAM_TYPE &o, AST_Expression::AST_ExprValue *ev)
     {
     case AST_Expression::EV_short:
       o << ev->u.sval;
-      break;
+      return;
     case AST_Expression::EV_ushort:
       o << ev->u.usval;
-      break;
+      return;
     case AST_Expression::EV_long:
       o << ev->u.lval;
-      break;
+      return;
     case AST_Expression::EV_ulong:
     case AST_Expression::EV_enum:
       o << ev->u.ulval;
-      break;
+      return;
     case AST_Expression::EV_float:
       o << ev->u.fval;
-      break;
+      return;
     case AST_Expression::EV_double:
       o << ev->u.dval;
-      break;
+      return;
     case AST_Expression::EV_char:
       o << ev->u.cval;
-      break;
+      return;
     case AST_Expression::EV_wchar:
       o << ev->u.wcval;
-      break;
+      return;
     case AST_Expression::EV_octet:
       {
         std::ios saved (nullptr);
@@ -2707,10 +2739,10 @@ dump_expr_val (ACE_OSTREAM_TYPE &o, AST_Expression::AST_ExprValue *ev)
         o << "0x" << std::hex << std::setw (2) << std::setfill ('0') << unsigned (ev->u.oval);
         o.copyfmt (saved);
       }
-      break;
+      return;
     case AST_Expression::EV_bool:
       o << (ev->u.bval == true ? "TRUE" : "FALSE");
-      break;
+      return;
     case AST_Expression::EV_string:
       if (ev->u.strval != nullptr)
         {
@@ -2720,25 +2752,32 @@ dump_expr_val (ACE_OSTREAM_TYPE &o, AST_Expression::AST_ExprValue *ev)
         {
           o << "(null string)";
         }
-      break;
+      return;
     case AST_Expression::EV_longlong:
       o << ev->u.llval;
-      break;
+      return;
     case AST_Expression::EV_ulonglong:
       o << ev->u.ullval;
-      break;
+      return;
     case AST_Expression::EV_fixed:
       o << ev->u.fixedval;
-      break;
+      return;
     case AST_Expression::EV_int8:
       o << static_cast<short> (ev->u.int8val);
-      break;
+      return;
     case AST_Expression::EV_uint8:
       o << static_cast<unsigned short> (ev->u.uint8val);
+      return;
+    case AST_Expression::EV_longdouble:
+    case AST_Expression::EV_wstring:
+    case AST_Expression::EV_any:
+    case AST_Expression::EV_object:
+    case AST_Expression::EV_void:
+    case AST_Expression::EV_none:
       break;
-    default:
-      o << "(Can not dump this type)";
     }
+
+    o << "(Can not dump type " << AST_Expression::exprtype_to_string (ev->et) << ")";
 }
 
 // Dump an AST_Expression node to the ostream o.
@@ -2840,6 +2879,7 @@ AST_Expression::dump (ACE_OSTREAM_TYPE &o)
         o << ACE_TEXT ("(nil symbolic name)");
       break;
     case EC_none:
+      o << ACE_TEXT ("(none)");
       break;
     default:
       o << ACE_TEXT ("unsupported dump mode for expression with ec == ")
@@ -3046,9 +3086,9 @@ AST_Expression::exprtype_to_string (ExprType t)
     return "uint8";
   case AST_Expression::EV_int8:
     return "int8";
-  default:
-    return "<UNKNOWN TYPE>";
   }
+
+  return "<UNKNOWN TYPE>";
 }
 
 AST_Enum *
diff --git a/TAO/tests/IDL_Test/IDL_Test.mpc b/TAO/tests/IDL_Test/IDL_Test.mpc
index 7ac4384b33d..5a2c335032b 100644
--- a/TAO/tests/IDL_Test/IDL_Test.mpc
+++ b/TAO/tests/IDL_Test/IDL_Test.mpc
@@ -26,6 +26,7 @@ project(*IDL): taoserver, messaging, gen_ostream {
     Bug_3312_Regression.idl
     Bug_3819_Regression.idl
     Bug_3821_Regression.idl
+    expressions.idl
     full.idl
     fwd.idl
     gperf.idl
@@ -147,6 +148,8 @@ project(*DLL): taoidldefaults, taolib, messaging {
     dif2S.cpp
     enum_in_structC.cpp
     enum_in_structS.cpp
+    expressionsC.cpp
+    expressionsS.cpp
     fullC.cpp
     fullS.cpp
     fwdC.cpp
diff --git a/TAO/tests/IDL_Test/expressions.idl b/TAO/tests/IDL_Test/expressions.idl
new file mode 100644
index 00000000000..50739eec980
--- /dev/null
+++ b/TAO/tests/IDL_Test/expressions.idl
@@ -0,0 +1,52 @@
+module ShortValues {
+  const short a = 6;
+  const short b = 3;
+  const short div = a / b;
+  const short mul = a * b;
+  const short add = a + b;
+  const short sub = a - b;
+  const short mod = a % b;
+};
+
+module LongValues {
+  const long a = 6;
+  const long b = 3;
+  const long div = a / b;
+  const long mul = a * b;
+  const long add = a + b;
+  const long sub = a - b;
+  const long mod = a % b;
+};
+
+module MixedIntValues {
+  const long div = LongValues::a / ShortValues::b;
+  const long mul = LongValues::a * ShortValues::b;
+  const long add = LongValues::a + ShortValues::b;
+  const long sub = LongValues::a - ShortValues::b;
+  const long mod = LongValues::a % ShortValues::b;
+};
+
+module FloatValues {
+  const float a = 6.0;
+  const float b = 3.0;
+  const float div = a / b;
+  const float mul = a * b;
+  const float add = a + b;
+  const float sub = a - b;
+};
+
+module DoubleValues {
+  const double a = 6.0;
+  const double b = 3.0;
+  const double div = a / b;
+  const double mul = a * b;
+  const double add = a + b;
+  const double sub = a - b;
+};
+
+module MixedFloatValues {
+  const double div = DoubleValues::a / FloatValues::b;
+  const double mul = DoubleValues::a * FloatValues::b;
+  const double add = DoubleValues::a + FloatValues::b;
+  const double sub = DoubleValues::a - FloatValues::b;
+};
diff --git a/TAO/tests/IDL_Test/main.cpp b/TAO/tests/IDL_Test/main.cpp
index 1fd8c57b4ef..60bfef43900 100644
--- a/TAO/tests/IDL_Test/main.cpp
+++ b/TAO/tests/IDL_Test/main.cpp
@@ -17,6 +17,7 @@
 #include "constantsC.h"
 #include "nested_scopeS.h"
 #include "typedefC.h"
+#include "expressionsC.h"
 
 #include "ace/Log_Msg.h"
 #include "ace/OS_NS_string.h"
@@ -79,6 +80,65 @@ struct something_handler
 {
 };
 
+template <typename Type>
+void
+expect_equals (int &error_count, const char *name, Type actual, Type expected)
+{
+  if (actual != expected)
+    {
+      *ACE_DEFAULT_LOG_STREAM
+        << "ERROR: For " << name << " expected: " << expected
+        << ", but got " << actual << "\n";
+      ++error_count;
+    }
+}
+
+void
+test_expressions (int &error_count)
+{
+  expect_equals<CORBA::Short> (error_count, "ShortValues::a", ShortValues::a, 6);
+  expect_equals<CORBA::Short> (error_count, "ShortValues::b", ShortValues::b, 3);
+  expect_equals<CORBA::Short> (error_count, "ShortValues::div", ShortValues::div, 2);
+  expect_equals<CORBA::Short> (error_count, "ShortValues::mul", ShortValues::mul, 18);
+  expect_equals<CORBA::Short> (error_count, "ShortValues::add", ShortValues::add, 9);
+  expect_equals<CORBA::Short> (error_count, "ShortValues::sub", ShortValues::sub, 3);
+  expect_equals<CORBA::Short> (error_count, "ShortValues::mod", ShortValues::mod, 0);
+
+  expect_equals<CORBA::Long> (error_count, "LongValues::a", LongValues::a, 6);
+  expect_equals<CORBA::Long> (error_count, "LongValues::b", LongValues::b, 3);
+  expect_equals<CORBA::Long> (error_count, "LongValues::div", LongValues::div, 2);
+  expect_equals<CORBA::Long> (error_count, "LongValues::mul", LongValues::mul, 18);
+  expect_equals<CORBA::Long> (error_count, "LongValues::add", LongValues::add, 9);
+  expect_equals<CORBA::Long> (error_count, "LongValues::sub", LongValues::sub, 3);
+  expect_equals<CORBA::Long> (error_count, "LongValues::mod", LongValues::mod, 0);
+
+  expect_equals<CORBA::Long> (error_count, "MixedIntValues::div", MixedIntValues::div, 2);
+  expect_equals<CORBA::Long> (error_count, "MixedIntValues::mul", MixedIntValues::mul, 18);
+  expect_equals<CORBA::Long> (error_count, "MixedIntValues::add", MixedIntValues::add, 9);
+  expect_equals<CORBA::Long> (error_count, "MixedIntValues::sub", MixedIntValues::sub, 3);
+  expect_equals<CORBA::Long> (error_count, "MixedIntValues::mod", MixedIntValues::mod, 0);
+
+  expect_equals<CORBA::Float> (error_count, "FloatValues::a", FloatValues::a, 6.0f);
+  expect_equals<CORBA::Float> (error_count, "FloatValues::b", FloatValues::b, 3.0f);
+  expect_equals<CORBA::Float> (error_count, "FloatValues::div", FloatValues::div, 2.0f);
+  expect_equals<CORBA::Float> (error_count, "FloatValues::mul", FloatValues::mul, 18.0f);
+  expect_equals<CORBA::Float> (error_count, "FloatValues::add", FloatValues::add, 9.0f);
+  expect_equals<CORBA::Float> (error_count, "FloatValues::sub", FloatValues::sub, 3.0f);
+
+  expect_equals<CORBA::Double> (error_count, "DoubleValues::a", DoubleValues::a, 6.0);
+  expect_equals<CORBA::Double> (error_count, "DoubleValues::b", DoubleValues::b, 3.0);
+  expect_equals<CORBA::Double> (error_count, "DoubleValues::div", DoubleValues::div, 2.0);
+  expect_equals<CORBA::Double> (error_count, "DoubleValues::mul", DoubleValues::mul, 18.0);
+  expect_equals<CORBA::Double> (error_count, "DoubleValues::add", DoubleValues::add, 9.0);
+  expect_equals<CORBA::Double> (error_count, "DoubleValues::sub", DoubleValues::sub, 3.0);
+
+  expect_equals<CORBA::Double> (error_count, "MixedFloatValues::div", MixedFloatValues::div, 2.0);
+  expect_equals<CORBA::Double> (error_count, "MixedFloatValues::mul", MixedFloatValues::mul, 18.0);
+  expect_equals<CORBA::Double> (error_count, "MixedFloatValues::add", MixedFloatValues::add, 9.0);
+  expect_equals<CORBA::Double> (error_count, "MixedFloatValues::sub", MixedFloatValues::sub, 3.0);
+
+}
+
 int
 ACE_TMAIN(int argc, ACE_TCHAR *argv[])
 {
@@ -415,5 +475,7 @@ ACE_TMAIN(int argc, ACE_TCHAR *argv[])
       return 1;
     }
 
-  return error_count;
+  test_expressions (error_count);
+
+  return error_count ? 1 : 0;
 }