From b79f031d852433b939bf19fc88b208955e4d4c55 Mon Sep 17 00:00:00 2001 From: Fred Hornsey Date: Thu, 4 Nov 2021 15:38:15 -0500 Subject: 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. --- TAO/TAO_IDL/ast/ast_expression.cpp | 216 ++++++++++++++++++++++--------------- TAO/tests/IDL_Test/IDL_Test.mpc | 3 + TAO/tests/IDL_Test/expressions.idl | 52 +++++++++ TAO/tests/IDL_Test/main.cpp | 64 ++++++++++- 4 files changed, 246 insertions(+), 89 deletions(-) create mode 100644 TAO/tests/IDL_Test/expressions.idl 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 +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 ( + 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 ( + 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 ( + 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 ( + 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 ( + 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 ( + 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 ( + 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 ( + pd_v1->ev ()->u.ullval, pd_v2->ev ()->u.ullval, retval->u.ullval); + break; + + case EV_octet: + success = do_eval_mod_op ( + pd_v1->ev ()->u.oval, pd_v2->ev ()->u.oval, retval->u.oval); + break; + + case EV_bool: + success = do_eval_mod_op ( + 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 (ev->u.int8val); - break; + return; case AST_Expression::EV_uint8: o << static_cast (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 ""; } + + return ""; } 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 +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 (error_count, "ShortValues::a", ShortValues::a, 6); + expect_equals (error_count, "ShortValues::b", ShortValues::b, 3); + expect_equals (error_count, "ShortValues::div", ShortValues::div, 2); + expect_equals (error_count, "ShortValues::mul", ShortValues::mul, 18); + expect_equals (error_count, "ShortValues::add", ShortValues::add, 9); + expect_equals (error_count, "ShortValues::sub", ShortValues::sub, 3); + expect_equals (error_count, "ShortValues::mod", ShortValues::mod, 0); + + expect_equals (error_count, "LongValues::a", LongValues::a, 6); + expect_equals (error_count, "LongValues::b", LongValues::b, 3); + expect_equals (error_count, "LongValues::div", LongValues::div, 2); + expect_equals (error_count, "LongValues::mul", LongValues::mul, 18); + expect_equals (error_count, "LongValues::add", LongValues::add, 9); + expect_equals (error_count, "LongValues::sub", LongValues::sub, 3); + expect_equals (error_count, "LongValues::mod", LongValues::mod, 0); + + expect_equals (error_count, "MixedIntValues::div", MixedIntValues::div, 2); + expect_equals (error_count, "MixedIntValues::mul", MixedIntValues::mul, 18); + expect_equals (error_count, "MixedIntValues::add", MixedIntValues::add, 9); + expect_equals (error_count, "MixedIntValues::sub", MixedIntValues::sub, 3); + expect_equals (error_count, "MixedIntValues::mod", MixedIntValues::mod, 0); + + expect_equals (error_count, "FloatValues::a", FloatValues::a, 6.0f); + expect_equals (error_count, "FloatValues::b", FloatValues::b, 3.0f); + expect_equals (error_count, "FloatValues::div", FloatValues::div, 2.0f); + expect_equals (error_count, "FloatValues::mul", FloatValues::mul, 18.0f); + expect_equals (error_count, "FloatValues::add", FloatValues::add, 9.0f); + expect_equals (error_count, "FloatValues::sub", FloatValues::sub, 3.0f); + + expect_equals (error_count, "DoubleValues::a", DoubleValues::a, 6.0); + expect_equals (error_count, "DoubleValues::b", DoubleValues::b, 3.0); + expect_equals (error_count, "DoubleValues::div", DoubleValues::div, 2.0); + expect_equals (error_count, "DoubleValues::mul", DoubleValues::mul, 18.0); + expect_equals (error_count, "DoubleValues::add", DoubleValues::add, 9.0); + expect_equals (error_count, "DoubleValues::sub", DoubleValues::sub, 3.0); + + expect_equals (error_count, "MixedFloatValues::div", MixedFloatValues::div, 2.0); + expect_equals (error_count, "MixedFloatValues::mul", MixedFloatValues::mul, 18.0); + expect_equals (error_count, "MixedFloatValues::add", MixedFloatValues::add, 9.0); + expect_equals (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; } -- cgit v1.2.1