| Ticket UUID: | 474335 | ||
| Title: | logic of matherr is backwards | ||
| Type: | Bug | Created on: | 2001-10-24 03:45:18 |
| Submitter: | dgp | Assigned to: | dgp |
| Subsystem: | 52. Portability Support | Severity: | |
| Priority: | 5 Medium | Last modified: | 2002-06-01 05:21:28 |
| Status: | Closed | Closed by: | dgp |
| Resolution: | Fixed | Closed on: | 2002-05-31 22:21:28 |
| Version: | obsolete: 8.4a4 | ||
| Description: | ||||
A simple look at the cvs history of
unix/tclMtherr.c shows the introduction of
a bug:
cvs diff -r core-8-0-5 -r core-8-1-0 tclMtherr.c
...
matherr(xPtr)
struct exception *xPtr; /* Describes error that
occurred. */
{
- if (!tcl_MathInProgress) {
+ if (TclMathInProgress()) {
return 0;
...
The "!" did not survive the transition from
access of a global variable to call of an internal
function. The logic of matherr() became reversed.
The obvious fix is to restore the proper logic.
It's worth pausing though and asking why this
bug hasn't been noticed in the 3 years since it
was introduced. I am unable to come up with
any script on any platform that exposes this error.
It seems that Tcl behaves the same whether the
logic is forwards or backwards.
It would seem then that Tcl would also behave the
same if the logic were completely removed. I welcome
second opinions on this, but my inclination would
be to remove the matherr() routine altogether
unless someone can demonstrate it's serving a
useful purpose somewhere. (And how useful can
it be if it's been wrong since Tcl 8.1.0 ?)
| ||||
| User Comments: | ||||
hobbs added on 2001-10-25 02:42:07:
Logged In: YES user_id=72656 And note that win/tclWinMtherr.c actually has the correct ! in that compat function. Note that it is a compat function. You'd like have to ask the older core guys (JohnO or Brian Lewis who did the original compiler) directly if/what meaning this had. hobbs added on 2001-10-25 02:43:24:
Logged In: YES user_id=72656 Of course, the comments for the Windows version say only Borland C++ uses it. However, that should be verified, as that used to be the official compiler of the sources, and the comment may be incorrect. dgp added on 2001-10-25 03:13:31:
Logged In: YES user_id=80530 Agreed that second opinions from the old-timers will be very helpful. I also note, that matherr() and _matherr() are the only callers of TclMathInProgress() so if they go, so can that. mdejong added on 2002-01-12 01:56:38:
Logged In: YES user_id=90858 I don't have anything useful to add, assigning this bug back to hobbs. jenglish added on 2002-01-16 09:21:46:
Logged In: YES user_id=68433 On the Unices that I have available, 'matherr()' is only used if you statically link against a special library instead of the default -lm (IRIX: -lmx; AIX: -lmsaa; Linux (RH 6.2, 7.0,7.1,7.2, SuSE 6.2): apparently not supported). A google search shows that matherr is supported on HP-UX and a few others, but the default behaviour (set errno and continue) is exactly the same as what's in unix/tclMtherr.c, so the latter is redundant. It would appear that this is either unused or redundant on all Unix platforms, and can safely be removed (except for one problem, see below). In unix/tclAppInit.c the following should also be taken out: /* * The following variable is a special hack that is needed in order for * Sun shared libraries to be used for Tcl. */ extern int matherr(); int *tclDummyMathPtr = (int *) matherr; This bit of code has been around since Tcl 7.4, but has long since been rendered obsolete by Stubs and newer versions of SunOS. The only problem with removing unix/tclMtherr.c is that all the custom tclsh's and wishes out there that have retained the tclDummyMathPtr definition for cargo-cult reasons will fail to link on platforms that don't have matherr(). It's an easy fix -- just get rid of the definition -- so this probably isn't much of an issue. dgp added on 2002-02-20 08:25:16:
Logged In: YES user_id=80530 davgrvy, can you comment on whether the _matherr() function in win/tclWinMtherr.c is still needed by Borland compilers? dgp added on 2002-02-22 00:31:28:
Logged In: YES user_id=80530 Since Tcl 8.4 should be released soon, now is not the time to risk breaking anything, but as soon as we get a HEAD aimed at 8.5a development I plan to yank out all this matherr business. jenglish added on 2002-05-14 04:50:27:
Logged In: YES user_id=68433 Another data point: On HP-UX 10.20, the function _matherr() is used when linking with the default math library (note leading underscore, NOT 'matherr()', which is what unix/tclMtherr.c defines). Also, the man page says the following: "The matherr() and _matherr() functions are obsolete and will not be supported at the next release of HP-UX [that would be HP-UX 11 -JE]. The matherr() function is not required by any version of XPG nor by ANSI C, and has not been part of any standard since SVID2. In the HP-UX math library, the SVID2 matherr() function has been renamed to _matherr(), and no error messages are printed to the standard error output. _matherr() is provided in libm.a in order to assist in supporting old programs. Executables that use the old matherr() or _matherr() will continue to run indefinitely, even on future releases of HP-UX." "However, if you intend to recompile or relink any of your current programs that use matherr() or _matherr() on a future release of HP-UX, you should be aware that any function called matherr() or _matherr() will no longer be called in case of library errors." davygrvy added on 2002-05-28 08:19:49:
Logged In: YES user_id=7549 Should this be assigned to me? I don't know how to follow this up. dgp added on 2002-05-28 10:50:05:
Logged In: YES user_id=80530 Hi, davygrvy, This was assigned to you looking for comments on whether the Borland compiler still had any need for a matherr() implementation. See the 2002-02-19 comment below. davygrvy added on 2002-05-28 11:04:49:
Logged In: YES
user_id=7549
There are 2 mentions of it in include/math.h:
struct _exception
{
int type;
char _FAR *name;
double arg1, arg2, retval;
};
struct _exceptionl
{
int type;
char _FAR *name;
long double arg1, arg2, retval;
};
typedef enum
{
DOMAIN = 1, /* argument domain error -- log (-1) */
SING, /* argument singularity -- pow (0,-2)) */
OVERFLOW, /* overflow range error -- exp (1000) */
UNDERFLOW, /* underflow range error -- exp (-1000)
*/
TLOSS, /* total loss of significance -- sin(10e70) */
PLOSS, /* partial loss of signif. -- not used */
STACKFAULT /* floating point unit stack overflow */
} _mexcep;
int _RTLENTRY _matherr(struct _exception *__e);
int _RTLENTRY _matherrl(struct _exceptionl
*__e);
I don't know what to say beyond the existence of it. I don't
have any documentation on Borland's RT to contribute any
useful concerns. fastmath.h says the following, though:
/*
fastmath.h - header file for the FastMath routines. These
are high
performance math routines that don't bother
checking for
most error conditions and never call matherr.
They are
coded for maximum speed and most are written in
assembly.
These functions are never exported from the
RTLDLL, which
means that they always get linked directly into the
PE file
which is being created.
*/
dgp added on 2002-05-29 23:47:54:
Logged In: YES user_id=80530 OK, thanks. I'll stick with the plan to remove all the matherr stuff from Tcl once we hit 8.5a development. jenglish added on 2002-05-30 00:34:45:
Logged In: YES user_id=68433 > I'll stick with the plan to remove all the matherr stuff from Tcl once we hit 8.5a development. Why not remove it now, while we're still in the 8.4 alpha series? See also patch #555635 "Dead code: matherr, TclMathInProgress". This is extremely unlikely to break anything. I realize that code cleanup is not a high priority -- "if it ain't broke, don't fix it" -- but dead code *does* have a cost. dgp added on 2002-06-01 05:21:28:
Logged In: YES user_id=80530 Patch 555635 accepted in Tcl 8.4a5. Fixed. Closed. | ||||
