pabru / picoc

Automatically exported from code.google.com/p/picoc
0 stars 0 forks source link

Shift operators ignore the unsigned type #200

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

1. Code example:

  unsigned long dwMask = 0xFFFFFFFF;
  dwMask >>= 1;
  printf("Result: 0x%08X\n", dwMask);

2. The displayed result will be:

  Result: 0xFFFFFFFF

3. The expected result is:

  Result: 0x7FFFFFFF

It looks that the "unsigned long" variable is treated as a "signed long" type.

In the file "expression.c", the problem can be fixed by casting all shift 
operators with "(unsigned long)":

/* evaluate an infix operator */
void ExpressionInfixOperator(struct ParseState *Parser, struct ExpressionStack 
**StackTop, enum LexToken Op, struct Value *BottomValue, struct Value *TopValue)
{
   // [...]

            case TokenShiftLeftAssign:      ResultInt = ExpressionAssignInt(Parser, BottomValue, (unsigned long) BottomInt << TopInt, FALSE); break;
            case TokenShiftRightAssign:     ResultInt = ExpressionAssignInt(Parser, BottomValue, (unsigned long) BottomInt >> TopInt, FALSE); break;
   // [...]

            case TokenShiftLeft:            ResultInt = (unsigned long) BottomInt << TopInt; break;
            case TokenShiftRight:           ResultInt = (unsigned long) BottomInt >> TopInt; break;

   // [...]
}

The above workaround works, but I agree that's not the best way to fix the 
problem. It would be better to check the TopValue and BottomValue variable 
types (signed/unsigned) and branch for the mathematical operation with the 
appropriate type (signed/unsigned). I suspect that all the other mathematical 
expressions are affected by this problem.

Original issue reported on code.google.com by mmass...@gmail.com on 15 Dec 2014 at 7:29

GoogleCodeExporter commented 9 years ago
I have a fix for this on GitHub 
(https://github.com/galacticstudios/picoc/commit/8018ff8708d1612c83498b4cdee722e
1b5735347)

Original comment by goo...@LoadAccumulator.com on 13 Aug 2015 at 11:37