Menu
▾
▴
[Tcl-bugs] [ tcl-Patches-509340 ] ClientData set to int* not void*
|
From: <no...@so...> - 2002-05-28 14:30:49
|
Patches item #509340, was opened at 2002-01-27 15:45 You can respond by visiting: http://sourceforge.net/tracker/?func=detail&atid=310894&aid=509340&group_id=10894 Category: 50. Other Tools Group: None Status: Open Resolution: None Priority: 5 Submitted By: David Gravereaux (davygrvy) Assigned to: Mo DeJong (mdejong) Summary: ClientData set to int* not void* Initial Comment: With the MS compiler, the following is allowed without casting even with langauge extensions turned off (-Za). /*#include "tcl.h"*/ #ifdef __STDC__ # if defined(__STDC_VERSION__) && __STDC_VERSION__ == 199409L # pragma message ("*** ISO C and AM1 complient") # else # pragma message ("*** ISO C, but not AM1 complient") # endif #else # pragma message ("*** Not ISO C complient") #endif void main (void) { /*ClientData someTypeLessPointer;*/ void *someTypeLessPointer; struct { __int64 a; char b; } someStruct; someTypeLessPointer = &someStruct; } Placing a typed pointer into a typeless pointer is allowed. The typedef for ClientData should be void*, not int* under this compiler, IMO. Notice the test for __STDC__ is a failure under this compiler, too. __STDC__ is only defined when the -Za switch is used. I'm at a loss to understand what the complience of the compiler really is, but getting back to the patch, a void* is legal, so let's use it. ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2002-05-28 07:30 Message: Logged In: NO Thinking about it, my previous proposal is probably too drastic - given that Jeff defends the sources like a lioness would defend her babies :) I just had a look at the sources of Tcl8.4 and noted that the test now looks like #if defined(__STDC__) || defined(__cplusplus) || defined(__BORLANDC__) So the logical thing to do would be to explicitely add VC++ there, too, via defined(MSCVER) - or similar.Regards Helmut ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2002-05-28 05:39 Message: Logged In: NO Currently ClientData is defined as follows (from tcl.h) # if defined(__STDC__) || defined(__cplusplus) typedef void *ClientData; # else typedef int *ClientData; # endif /* __STDC__ */ With the Borland compiler (and apparently with VC++, too) this will result in" typedef int *ClientData", leading to a lot of otherwise unnecessary typecasts. Explanation: For the Borland compiler __STDC__ means *strict* compliance to ANSI C (you can set this mode with the -A flag). With this flag turned on, however, Tcl won't compile - some Windows header files containing C++ style (//) comments. I tend to think that this is a sensible thing to do (you want ANSI C, you get - strict - ANSI C), but this is just opinion - we got a problem to solve. The criterion used above # if defined(__STDC__) || defined(__cplusplus) assumes this scenario: - pre-ANSI C - ANSI C or - C++ However this does not describe the actual situation, where we have sort of a post-ANSI period: C compilers adhering to the standard but allowing extensions, too (like C++ comments). (And differing of course in whether they #define __STDC__ for these extensions or not.). I would therefore suggest that we change the criterion. After all, all we want to know is whether the compiler knows the 'void' key word. We don't really care (at this point) whether its internal logic defines __STDC__. So my suggestion is to add a #define KNOSW_VOID and use this for the typedef of ClientData # ifdef KNOWS_VOID typedef void *ClientData; # else typedef int *ClientData; # endif /* KNOWS_VOID */ I assume that all current compilers support 'void', so we probably don't even need this check. If, however, it turns out that there is a compiler which barks on 'void', we could add stuff to handle just this compiler. #define KNOWS_VOID #if defined SomeWeirdCompiler #undef KNOWS_VOID #endif In any case, we should change the criterion in some way, because it doesn't handle the actual situation. Just my 0.02 Helmut Giese ---------------------------------------------------------------------- Comment By: Jeffrey Hobbs (hobbs) Date: 2002-03-04 21:41 Message: Logged In: YES user_id=72656 Can someone tell me why this needs to be applied? I still fail to see a problem with the current sources. ---------------------------------------------------------------------- Comment By: David Gravereaux (davygrvy) Date: 2002-03-04 21:07 Message: Logged In: YES user_id=7549 MinGW gets a void*, why can't VC++? Notice -D__STDC__=1 below. VC++ doesn't set that. It only sets it under strict -Za, which not having it defined doesn't mean less than, which it isn't. That's like if gcc only set - D__STDC__=1 *only* when -pedantic -ansi was used. The current logic of looking at the __STDC__ #define is missleading under VC++. c:\dev\mingw\bin\..\lib\gcc-lib\mingw32\2.95.3-5\cpp0.exe - lang-c -v -iprefix c:\dev\mingw\bin\../lib/gcc- lib/mingw32/2.95.3-5/ -D__GNUC__=2 -D__GNUC_MINOR__=95 - Di386 -D_WIN32 -DWIN32 -D__WIN32__ -D__MINGW32__=1.0 - D__MSVCRT__ -DWINNT -D_X86_=1 -D__STDC__=1 - D__stdcall=__attribute__((__stdcall__)) -D_stdcall=__attrib ute__((__stdcall__)) -D__cdecl=__attribute__((__cdecl__)) - D__declspec(x)=__attribute__((x)) -D__i386__ -D_WIN32 - D__WIN32__ -D__WIN32__ -D__MINGW32__=1.0 -D__MSVCRT__ - D__WINNT__ -D_X86_=1 -D__STDC__=1 -D__stdcall=__attribute__ ((__stdcall__)) -D___stdcall__=__attribute__ ((__stdcall__)) -D__cdecl=__attribute__((__cdecl__)) - D__declspec(x)=__attribute__((x)) -D__i386 -D__WIN32 - D__WINNT -D___stdcall=__attribute__((__stdcall__)) -Asystem (winnt) -Acpu(i386) -Amachine(i386) -remap -Acpu(i386) - Amachine(i386) -Di386 -D__i386 -D__i386__ --help help-dummy C:\DOCUME~1\daveg\LOCALS~1\Temp\ccGGaaaa.i ---------------------------------------------------------------------- Comment By: David Gravereaux (davygrvy) Date: 2002-02-25 12:44 Message: Logged In: YES user_id=7549 This is not the source, it's the typedef. It doesn't make sense. Why on earth is an int* used when a void* can be and is easy to work with. I hate casting when it isn't needed. VC++ doesn't need to cast, as it understand what a void* is. What does: #if defined(__STDC__) || defined(__cplusplus) mean to you? Does it mean your compiler doesn't understand a void*, or we are going to force you to cast in both directions even if your compiler doesn't need to? ---------------------------------------------------------------------- Comment By: Jeffrey Hobbs (hobbs) Date: 2002-02-25 11:02 Message: Logged In: YES user_id=72656 I fail to see a problem stated. If it ain't broke, don't fix it. Unless there is a clear problem with the current source, let's not change things. ---------------------------------------------------------------------- Comment By: David Gravereaux (davygrvy) Date: 2002-02-22 23:53 Message: Logged In: YES user_id=7549 forget "some systems". This is Win32 with MSVC as the compiler. What is that test asking? Is it asking are you strict ANSI or ANSI compilent as oppossed to pre-ANSI? #ifndef _CLIENTDATA # if defined(__STDC__) || defined(__cplusplus) || defined (__BORLANDC__) typedef void *ClientData; # else typedef int *ClientData; # endif /* __STDC__ */ # define _CLIENTDATA #endif More flexible? How so? sizeof(*(ClientData))? What good is that, not that it would work? I do most of my work in C++ and never had to cast a typed pointer to a ClientData. Coming out of a ClientData to a typed pointer, of course yes, I had to cast. Then I do some work in C, and I get this damn cast shit going into a ClientData. But its still legal in C. Typed pointer to a void pointer without a cast is legal in this compiler for C. If this is the case, and is fine in C++ according to the existing logic, don't force an int* when it isn't needed, please. There are users of the Tcl API besides the core itself where old-fashioned seems to always take precidence over advances. Just because its there, and has been for a while, doesn't mean its right by only it's existence. Please question the meaning of the preprocessor logic. It doesn't make sense for this compiler. #if !defined(__STDC__) and !defined(__cplusplus) == 1 does that mean a void* isn't legal? But in the case for defined(_MSC_VER), it is true, though. Is that test testing a compiler feature or forcing an interface? If it's forcing an interface, why? It gets in my way as a user of the API. ---------------------------------------------------------------------- Comment By: Jeffrey Hobbs (hobbs) Date: 2002-02-22 23:04 Message: Logged In: YES user_id=72656 I'm not sure that I see this is really a problem in the sources though ... also int* gives more flexibility than void* on some systems. ---------------------------------------------------------------------- Comment By: David Gravereaux (davygrvy) Date: 2002-01-27 16:07 Message: Logged In: YES user_id=7549 In this case, I guess "not ISO C" means greater than instead of "sub ISO C" ---------------------------------------------------------------------- You can respond by visiting: http://sourceforge.net/tracker/?func=detail&atid=310894&aid=509340&group_id=10894 |