Tcl Source Code

View Ticket
Login
Ticket UUID: 1230205
Title: expr: div and mod support for LONG_MIN and LONG_MAX
Type: Patch Version: None
Submitter: mdejong Created on: 2005-06-30 08:15:45
Subsystem: 47. Bytecode Compiler Assigned To: dgp
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2005-07-12 06:07:48
Resolution: Accepted Closed By: dgp
    Closed on: 2005-07-11 17:10:12
Description:
Tcl's expr divide and modulus operations are buggy WRT
LONG_MIN and LONG_MAX values. The attached patch
fixes the problems, adds test cases, and reimplements the
divide operation so that it will execute more quickly. This
patch has been tested on a 32 bit long system. It
should also
work on a system with a 64 bit long, but that has not been
tested. This is a pure bug fix so it should not require
a TIP.
User Comments: kennykb added on 2005-07-12 06:07:48:
Logged In: YES 
user_id=99768

OK, got it, and 1229765 deals with the (nearly unavoidable)
problem
of parsing -0x80000000 as two tokens, '-' and '0x80000000'
-- with
the latter causing an overflow to a wide.  As dgp says,
1229765 is a fine workaround, and TIP #249 should be a 
permanent fix.  I'm satisfied.

mdejong added on 2005-07-12 06:01:09:
Logged In: YES 
user_id=90858

Actually, you are thinking of patch 1229765. This patch does
not have anything to do with parsing of expr literals. This
patch only changes
the way that the % and / expr operators are implemented.
Discussion of
implementation for that patch should appear elsewhere.

kennykb added on 2005-07-12 05:47:40:
Logged In: YES 
user_id=99768

Uhm.  I wasn't able to give this proper attention while I was
on vacation, which is when all the real action happened.
But wasn't one of the issues that the LONG_MIN value
was incorrectly made 'wide' by the parser?  And doesn't
it make sense to fix that one in the parser rather than the
bytecode engine?

Or do I have this confused with a different bug?

mdejong added on 2005-07-09 07:28:29:
Logged In: YES 
user_id=90858

Added to CVS on 2005-07-08.

mdejong added on 2005-07-02 04:31:56:
Logged In: YES 
user_id=90858

Well, this is not a number parsing bug. It is a problem with
Tcl's implementation of the divide and modulus operators in
the expr
code. The code in question lives in the bytecode execution
module,
but it really part of the functionality covered by the expr
command.

dgp added on 2005-07-02 02:16:16:
Logged In: YES 
user_id=80530


I'd like to look into whether this
can be corrected more easily
in the expr parser instead of 
the bytecode execution engine.

mdejong added on 2005-07-01 08:23:46:

File Added - 140440: expr2.patch

mdejong added on 2005-07-01 08:23:45:
Logged In: YES 
user_id=90858

I did some more testing today and found a problem. I have
attached an
updated patch that should be the final form. As far as 32 vs
64 bit tests go, I think that expr.test will work for both
32 bit machines with long and long long types and 64 bit
long systems. I have only tested on 32bit long systems.

hobbs added on 2005-07-01 06:44:55:
Logged In: YES 
user_id=72656

A bug is a bug, and it should go into 8.4 if the patch is
correct for it.  Should I test these specifically on systems
with 64-bit long (eg, Solaris 64-bit), or more extensive
testing of some other form?

mdejong added on 2005-07-01 03:21:09:
Logged In: YES 
user_id=90858

I don't see how TIP 249 is going to have any effect on this
issue.
My reading of TIP 249 indicates that it details how to parse
numbers into internal representations. This patch deals with
division and modulus implementation details after the number has
been converted into a long int or long long integer. This
patch fixes
bugs in Tcl's implementation of division and modulus that
show up
when the largest or smallest integers for a given type are
used. Also,
I do intend to check these changes into Tcl 8.4.

dgp added on 2005-06-30 21:05:28:
Logged In: YES 
user_id=80530


Unless this is targeted at 8.4.x, seems
it would be easier just to let TIP 249
make these corrections.

mdejong added on 2005-06-30 15:15:47:

File Added - 140329: expr.patch

Attachments: