Bug in Ternary Operator

#1 Huh, interesting.

it would also be interesting to print %neg itself as well.
is it -1, or 4.29497e+009 ?

i would guess something is casting an int to an unsigned int.

#2 Just tested it.

It's not the ternary that is causing the problem. It's the

%neg * 10

that is causing the problem.

I wonder why.
OK ... I might just share what I found

the ternary is returning a unsigned int

so doing this

%neg = false ? 1 : -1;

Will make "%neg" a unsigned int

You can fix this by

if(false)
      %neg = 1 ;
   else
      %neg = -1 ;

Aun

EDIT** Add what I found''' I must be having alot of free time :)

#3 Hum... well it seems to be that the ternary operator in TorqueScript needs fixing. Anyone want to take a crack at this?

#4 Confirmed definite weirdness here.

==>$a = false ? 1 : -1;
==>echo ($a);
-1
==>echo($a * 1);
4.29497e+009
==>$b = $a;
==>echo($b);
-1
==>echo($b * 1);
-1

#5 This is definitely beyond me,
but i can post where i got in case anyone else wants to pick up the ball.

what's going on is in

U32 ConditionalExprNode::compile(U32 *codeStream, U32 ip, TypeReq type)

TypeReq in this case refers to the apparent type of the "true" option,
which in this case is TypeReqUInt.

The problem is that the type of the first argument is being used to interpret both arguments,
and "1" is TypeReqUInt, while "-1" is TypeReqFloat.

I tried promoting TypeReqUInt to TypeReqFloat, but it complained.

It seems to me that ConditionExprNode::compile() needs to take two type arguments instead of one:
TypeReq typeTrue, TypeReq typeFalse.


however,
i did find a couple workarounds which allow you to still use the ternary.
all of the following will yield the result you want.

make the arguments floats:

$a = false ? 1.0 : -1.0;

make the arguments strings:

$a = false ? "1" : "-1";

put the negative argument first and flip the conditional:

$a = [b]![/b]false ? "-1" : "1";

and uncovered another bug with the ternary which results from the same oversimplifaction of types:

==>$a = false ? -1 : "foo";
==>echo($a);
0

#6 Does GG ever put a bounty (money or otherwise) on a bug? Just wondering...

#7 Not that i know of.
you certainly could, tho! ;)

#8 Here's a possible solution. Replace ConditionalExprNode::getPreferredType() in astNodes.cc with this:

TypeReq ConditionalExprNode::getPreferredType()
{
   TypeReq  trueType = trueExpr->getPreferredType();
   TypeReq  falseType = falseExpr->getPreferredType();

   if ( trueType == TypeReqString || falseType == TypeReqString )
      return TypeReqString;
      
   if ( trueType == TypeReqFloat || falseType == TypeReqFloat )
      return TypeReqFloat;
      
   return TypeReqUInt;
}

This promotes the type of the result to string if either the true or false expressions are of type string, or to float if either of the true or false expressions are of type float.

Let me know how that works for you...

#9 Hey Andy - thanks !
i'll try it out monday.

#10 Andy - that seems to fix all the use-cases i can throw at it.
nice one!

#11 Woops - very early heads up,
but we think there *may* be a problem with this change. ie, unexpected results.
will investigate more.

#12 Hi Andy,
sorry for the delay getting back on this.

okay, so i'm way out of my depth here,
but it seems like what was happenin' was during compilation,
ConditionalExprNode::getPreferredType() was being called with trueType and falseType both = TypeReqNone,
which resulted in TypeReqNone being promoted to TypeReqUInt,
and when the actual code was executed, trueExpr was say "1 2 3 4" and would thus be promoted to "1".

now,
i have no clue at all how/why the types of the arguments are being evaluated at *compile time*,
because this is an late-typed scripting language, so omgwtf.

anyhow,
i changed your code to default to the preferredType of trueExpr() instead of defaulting to TypeReqUInt.
also reversed the order of lines a bit in order have the debug prints at the bottom and preserve the precedents of promotion.

so the code below fixes the example above in "Confirmed definite weirdness here",
and also seems to work w/ our codebase.

static const char* TypeReqAsString(TypeReq tr)
{
   const char* ret = "unknown";

   switch (tr)
   {
      case TypeReqNone:
         ret = "TypeReqNone";
         break;
      case TypeReqUInt:
         ret = "TypeReqUInt";
         break;
      case TypeReqFloat:
         ret = "TypeReqFloat";
         break;
      case TypeReqString:
         ret = "TypeReqString";
         break;
   }

   return ret;
}


TypeReq ConditionalExprNode::getPreferredType()
{
   TypeReq  trueType  = trueExpr ->getPreferredType();
   TypeReq  falseType = falseExpr->getPreferredType();
   TypeReq  ret;

   if (true)
      ret = trueType;

   if (trueType == TypeReqFloat  || falseType == TypeReqFloat )
      ret = TypeReqFloat;

   if (trueType == TypeReqString || falseType == TypeReqString)
      ret = TypeReqString;
      
// Con::errorf("trueType  = %d - %s", trueType , TypeReqAsString(trueType ));
// Con::errorf("falseType = %d - %s", falseType, TypeReqAsString(falseType));
// Con::errorf("return    = %d - %s", ret      , TypeReqAsString(ret      ));

   return ret;
}

#13 Fwiw, i'm now considering just

TypeReq ConditionalExprNode::getPreferredType()
{
    if (trueExpr->getPreferredType() == falseExpr->getPreferredType())
       return trueExpr->getPreferredType();
    else
       return TypeReqNone;
}