[Tcl-bugs] [ tcl-Bugs-1104873 ] missing TCL_STORAGE_CLASS ?

[SourceForge.net <no...@so...>]

Bugs item #1104873, was opened at 2005-01-18 15:02
Message generated for change (Comment added) made by jenglish
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1104873&group_id=10894

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: 52. Configuration and Build Tools
Group: obsolete: 8.4.9
>Status: Closed
>Resolution: Rejected
Priority: 5
Private: No
Submitted By: Don Porter (dgp)
Assigned to: Joe English (jenglish)
Summary: missing TCL_STORAGE_CLASS ?

Initial Comment:

When #include-ing both tcl.h and tk.h,
a colleague notices this apparently
inconsistent set of declarations:

...

  #ifdef __cplusplus
  #   define EXTERN extern "C" TCL_STORAGE_CLASS
  #else
  #   define EXTERN extern TCL_STORAGE_CLASS
  #endif

...

  typedef int (Tcl_PackageInitProc)
               _ANSI_ARGS_((Tcl_Interp *interp));

...

  EXTERN void Tcl_StaticPackage 
             _ANSI_ARGS_((Tcl_Interp * interp, 
             char * pkgName, Tcl_PackageInitProc *
initProc, 
             Tcl_PackageInitProc * safeInitProc));

...

   EXTERN int Tk_Init _ANSI_ARGS_((Tcl_Interp * interp));
   EXTERN int Tk_SafeInit _ANSI_ARGS_((Tcl_Interp *
interp));


Colleague claims this inconsistency shows up
when the Watcom compiler tries to compile

  Tcl_StaticPackage(interp, "Tk", Tk_Init, Tk_SafeInit);

with complaints of a type mismatch between
what Tcl_StaticPackage expects and what
the Tk_*Init routines are declared to be.

Further claim is that all would be well if the
Tcl_PackageInitProc typedef were changed to:

    typedef TCL_STORAGE_CLASS 
               int (Tcl_PackageInitProc)
               _ANSI_ARGS_((Tcl_Interp *interp));

I don't buy it; but I also don't understand this
mess well enough to explain what might be
wrong with his proposal.  Help?

----------------------------------------------------------------------

>Comment By: Joe English (jenglish)
Date: 2008-02-23 09:11

Message:
Logged In: YES 
user_id=68433
Originator: NO

"typedef TCL_STORAGE_CLASS int (Tcl_PackageInitProc)(Tcl_Interp *interp);"
would be incorrect for the standard definition of TCL_STORAGE_CLASS on all
supported platforms (MSVC, GCC on Windows, and for all non-Windows
platforms where TCL_STORAGE_CLASS is simply "extern".)

If any Watcom users can suggest a solution that's compatible with Tcl's
"officially-supported" compiler/platform combinations, it's worth
considering, but this isn't it.

----------------------------------------------------------------------

Comment By: Michael Donahue (mjdonahue)
Date: 2005-02-02 19:54

Message:
Logged In: YES 
user_id=1210132

The watcom compiler complained about
Tcl_StaticPackage/Tcl_PackageInitProc but not
Tcl_CreateCommand/Tcl_CmdProc because Tk_Init was used as an
argument to Tcl_StaticPackage, and Tk_Init is declared
elsewhere in the Tk header files with a different type.

However, on reflection, I think all the Tcl_*Proc typedef's
should include some storage class info.  The watcom compiler
didn't complain about these because these typedefs are not
being applied to functions declared inside the Tcl library,
but rather to user code.  But, the Tcl library is going to
make calls into these functions, and it was built with
certain assumptions about the calling convention for these
user defined functions.  So it seems that information should
be included in all the Tcl_*Proc typedefs.

Maybe one shouldn't use the TCL_STORAGE_CLASS macro here,
since these are functions outside the Tcl library, but
instead define a new macro, something like
FOREIGN_STORAGE_CLASS.  This new macro would *not* include
the dllimport/dllexport qualifiers found in
TCL_STORAGE_CLASS, but would include things like cdecl.  In
this regard, check out the comment preceding the definition
of Tcl_AppInit near the bottom of tcl.h.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2005-02-02 06:35

Message:
Logged In: YES 
user_id=80530


Is there any explanation for why
the watcom compiler complains
about the combination of
Tcl_StaticPackage / Tcl_PackageInitProc
but allows the combination of
Tcl_CreateCommand / Tcl_CmdProc ?

----------------------------------------------------------------------

Comment By: David Gravereaux (davygrvy)
Date: 2005-02-02 01:40

Message:
Logged In: YES 
user_id=7549

I'll have a look at adding __cdecl.  It shouldn't be hard to do.
It sort-of is implicit, but nothing is wrong with explicity
except that our protos will get more wordy than they already
are..  There might be a WATCOM specific #pragma to fix it
globally..  I'll have a look.

Thanks for the detail.

----------------------------------------------------------------------

Comment By: Michael Donahue (mjdonahue)
Date: 2005-02-01 19:58

Message:
Logged In: YES 
user_id=1210132

There are two issues here.

ISSUE 1:  Currently, the Tcl_PackageInitProc typedef is

   typedef int (Tcl_PackageInitProc) _ANSI_ARGS_((Tcl_Interp
*interp));

I suggest that this is incorrect, and should more properly be

   typedef TCL_STORAGE_CLASS int (Tcl_PackageInitProc)
_ANSI_ARGS_((Tcl_Interp *interp));

Referring to the URL mentioned by davygrvy,
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/
vccelng/htm/msmod_17.asp, the full sentence reads:

Unlike the __near and __far keywords, which actually affect
the type of object or function (in this case, 2- and 4-byte
addresses), these storage-class attributes do not redefine
the type attributes of the object itself.

The reference here is to a particular subset of __declspec
modifiers (including dllimport and dllexport).  This would
appear to imply that other storage-class attributes may
redefine the type attribute.  For this reason, I believe
that TCL_STORAGE_CLASS should be included in the
Tcl_PackageInitProc typedef.  Currently, TCL_STORAGE_CLASS
is defined to be either empty or
__declspec(dllimport/dllexport).  Either way, this shouldn't
affect the type attribute (as pointed out by davygrvy), so
this change shouldn't break anything.

ISSUE 2: The Open Watcom compiler uses by default a
different calling convention than Microsoft VC.  Open Watcom
does support the VC calling convention, but you have to
specify when a function is using that convention.  This does
change the type attribute, so it seems sensible to put that
information into the TCL_STORAGE_CLASS macro.  I am
uncertain as to the best way to do this, but I find that if
I put

   __declspec(__cdecl)

into TCL_STORAGE_CLASS, then along with the change suggested
in ISSUE 1 above, I can successfully link code compiled with
Open Watcom against a Tcl DLL built with VC.  Of course,
this is a Watcom specific extension, so it should be wrapped
with #if defined(__WATCOMC__) guards.  This seems reasonable
given the compiler-specific code already in the
DLLIMPORT/DLLEXPORT definitions.

Actually, I need __declspec(__cdecl) to link against
anything in the Tcl DLL's.  Making only this change my code
builds completely *except* for the problem addressed in
ISSUE 1, which looks like a bug to me regardless of whether
one wants to add Watcom support.  ISSUE 2 is clearly a
Watcom compiler specific matter.


----------------------------------------------------------------------

Comment By: David Gravereaux (davygrvy)
Date: 2005-01-27 10:01

Message:
Logged In: YES 
user_id=7549

This might be related:
http://bugzilla.openwatcom.org/show_bug.cgi?id=35

----------------------------------------------------------------------

Comment By: David Gravereaux (davygrvy)
Date: 2005-01-27 09:56

Message:
Logged In: YES 
user_id=7549

As per
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccelng/htm/msmod_17.asp:
"
....storage-class attributes do not redefine the type
attributes of the object itself.
"

Still seems like a Watcom compiler bug to me.

----------------------------------------------------------------------

Comment By: David Gravereaux (davygrvy)
Date: 2005-01-27 09:48

Message:
Logged In: YES 
user_id=7549

Sounds like Watcom doesn't view a prototype with an import
attribute to be equal to one with.  Does where it come from
really make it part of the signature?

Are we going to have to go back to .def files or what?  VC++
doesn't get confused by this.  Sounds like a compiler bug.

__declspec(__dllimport) int foo (int a);
int bar (int a);

The sigs for type comparison are identical.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2005-01-26 06:41

Message:
Logged In: YES 
user_id=80530

To my knowledge, there is not
file tcl84s.lib distributed as
part of ActiveTcl, so he's trying
to link against the import
library tcl84.lib.  He does
succeed in the link, but had
to make changes regarding
TCL_STORAGE_CLASS to
get the compile to go, I think.

----------------------------------------------------------------------

Comment By: David Gravereaux (davygrvy)
Date: 2005-01-25 20:24

Message:
Logged In: YES 
user_id=7549

1) The watcom compiler *IS* a windows (and DOS) compiler.

2) When building projects that include tcl.h as a client
(aka building a custom shell), the default storage class is
for import from the DLL (tcl84.dll).  By defining
STATIC_BUILD the storage class becomes empty as when one
will link with tcl84s.lib (the static library).

Is your colleague targeting for tcl84s.lib (the static) or
tcl84.lib (the export)?  If he is targeting for tcl84s.lib
(the static), then do compile with -DSTATIC_BUILD or the
linker will become confused.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2005-01-24 10:46

Message:
Logged In: YES 
user_id=80530

Haven't got a report back yet,
but I don't see how setting
-DSTATIC_BUILD is going
to help resolve the conflicting
calling conventions of the 
two compilers.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2005-01-24 10:43

Message:
Logged In: YES 
user_id=80530

FWIW, it appears the STATIC_BUILD
setting has no impact on non-Windows
systems.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2005-01-19 07:37

Message:
Logged In: YES 
user_id=80530

Ah.  Maybe we're getting somewhere.

Where can I read about this
STATIC_BUILD and when
and how it should be used?

----------------------------------------------------------------------

Comment By: David Gravereaux (davygrvy)
Date: 2005-01-18 23:01

Message:
Logged In: YES 
user_id=7549

putting TCL_STORAGE_CLASS in the typedef isn't a bad idea,
actually.  Yes, it's messy stuff.

Trouble is, whether the function is being exported,
imported, or have a static association _should_ have no
bearing on the usage...

What I find odd is that Tcl_StaticPackage is mentioned, but
is he importing the Tk_*Init symbols??  That's strange.. 
Did he forget to set the -DSTATIC_BUILD macro?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1104873&group_id=10894