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 |