You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@stdcxx.apache.org by fa...@apache.org on 2007/09/18 19:56:45 UTC
svn commit: r577000 - in /incubator/stdcxx/trunk: include/loc/_messages.h
src/messages.cpp
Author: faridz
Date: Tue Sep 18 10:56:44 2007
New Revision: 577000
URL: http://svn.apache.org/viewvc?rev=577000&view=rev
Log:
2007-09-18 Farid Zaripov <Fa...@epam.com>
* _messages.h (__rw_cat_open): Declare fnuction as _RWSTD_EXPORT.
* _messages.h (__rw_get_message): Ditto.
* _messages.h (__rw_get_locale): Ditto.
* _messages.h (__rw_cat_close): Ditto.
* messages.cpp (__rw_cat_open): Ditto.
* messages.cpp (__rw_get_message): Ditto.
* messages.cpp (__rw_get_locale): Ditto.
* messages.cpp (__rw_cat_close): Ditto.
Modified:
incubator/stdcxx/trunk/include/loc/_messages.h
incubator/stdcxx/trunk/src/messages.cpp
Modified: incubator/stdcxx/trunk/include/loc/_messages.h
URL: http://svn.apache.org/viewvc/incubator/stdcxx/trunk/include/loc/_messages.h?rev=577000&r1=576999&r2=577000&view=diff
==============================================================================
--- incubator/stdcxx/trunk/include/loc/_messages.h (original)
+++ incubator/stdcxx/trunk/include/loc/_messages.h Tue Sep 18 10:56:44 2007
@@ -49,13 +49,17 @@
_RWSTD_NAMESPACE (__rw) {
-int __rw_cat_open (const _STD::string&, const _V3_LOCALE::locale&);
+_RWSTD_EXPORT int
+__rw_cat_open (const _STD::string&, const _V3_LOCALE::locale&);
-const char* __rw_get_message (int, int, int);
+_RWSTD_EXPORT const char*
+__rw_get_message (int, int, int);
-const _V3_LOCALE::locale& __rw_get_locale (int);
+_RWSTD_EXPORT const _V3_LOCALE::locale&
+__rw_get_locale (int);
-void __rw_cat_close (int);
+_RWSTD_EXPORT void
+__rw_cat_close (int);
} // namespace __rw
Modified: incubator/stdcxx/trunk/src/messages.cpp
URL: http://svn.apache.org/viewvc/incubator/stdcxx/trunk/src/messages.cpp?rev=577000&r1=576999&r2=577000&view=diff
==============================================================================
--- incubator/stdcxx/trunk/src/messages.cpp (original)
+++ incubator/stdcxx/trunk/src/messages.cpp Tue Sep 18 10:56:44 2007
@@ -210,7 +210,7 @@
// Open a message catalog and assign and return a handle for it.
-int
+_RWSTD_EXPORT int
__rw_cat_open (const _STD::string &cat_name, const _STD::locale &loc)
{
_RWSTD_MT_CLASS_GUARD (__rw_open_cat_data);
@@ -233,7 +233,7 @@
// Get message text from catalog.
-const char*
+_RWSTD_EXPORT const char*
__rw_get_message (int cat, int set_num, int msg_num)
{
if (cat < 0)
@@ -261,7 +261,7 @@
// Get locale to be used for character translation for this message catalog.
-const _STD::locale&
+_RWSTD_EXPORT const _STD::locale&
__rw_get_locale (int cat)
{
_RWSTD_MT_CLASS_GUARD (__rw_open_cat_data);
@@ -276,7 +276,7 @@
// Close a catalog and release its handle.
-void
+_RWSTD_EXPORT void
__rw_cat_close (int cat)
{
_RWSTD_MT_CLASS_GUARD (__rw_open_cat_data);
Re: svn commit: r577000 - in /incubator/stdcxx/trunk: include/loc/_messages.h
src/messages.cpp
Posted by Martin Sebor <se...@roguewave.com>.
Farid Zaripov wrote:
>> -----Original Message-----
>> From: Martin Sebor [mailto:sebor@roguewave.com]
>> Sent: Wednesday, September 19, 2007 2:09 AM
>> To: stdcxx-dev@incubator.apache.org
>> Subject: Re: svn commit: r577000 - in
>> /incubator/stdcxx/trunk: include/loc/_messages.h src/messages.cpp
>>
>> How about this: I'd like us to outline the functions
>> regardless of these errors because I believe it's an
>> improvement (I've done it for most other facets and classes
>> in the library so we have a good precedent :)
>>
>> I've outlined them here:
>> http://svn.apache.org/viewvc?rev=577098&view=rev
>>
>> Could you try, as an experiment, to revert the dllexport
>> change to see if the linker errors go away?
>
> The errors away.
> http://svn.apache.org/viewvc?rev=577304&view=rev
>
> I think that theese functions could be static and their prototypes
> could be
> removed from _messages.h since they are invoked from messages.cpp only.
I would like them to be, but they are referenced from the member
functions of the template defined in _messages.cc. When explicit
instantiation isn't enabled, they would not be found, would they?
We could solve it by explicitly specializing the virtual members
on char and wchar_t instead of explicitly instantiating them. I
think that would be good approach in general, one that we might
want to consider for all the facets in the future (it's too late
for it now).
Martin
RE: svn commit: r577000 - in /incubator/stdcxx/trunk: include/loc/_messages.h src/messages.cpp
Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Martin Sebor [mailto:sebor@roguewave.com]
> Sent: Wednesday, September 19, 2007 2:09 AM
> To: stdcxx-dev@incubator.apache.org
> Subject: Re: svn commit: r577000 - in
> /incubator/stdcxx/trunk: include/loc/_messages.h src/messages.cpp
>
> How about this: I'd like us to outline the functions
> regardless of these errors because I believe it's an
> improvement (I've done it for most other facets and classes
> in the library so we have a good precedent :)
>
> I've outlined them here:
> http://svn.apache.org/viewvc?rev=577098&view=rev
>
> Could you try, as an experiment, to revert the dllexport
> change to see if the linker errors go away?
The errors away.
http://svn.apache.org/viewvc?rev=577304&view=rev
I think that theese functions could be static and their prototypes
could be
removed from _messages.h since they are invoked from messages.cpp only.
Farid.
Re: svn commit: r577000 - in /incubator/stdcxx/trunk: include/loc/_messages.h
src/messages.cpp
Posted by Martin Sebor <se...@roguewave.com>.
Farid Zaripov wrote:
>> -----Original Message-----
>> From: Martin Sebor [mailto:sebor@roguewave.com]
>> Sent: Tuesday, September 18, 2007 10:24 PM
>> To: stdcxx-dev@incubator.apache.org
>> Subject: Re: svn commit: r577000 - in
>> /incubator/stdcxx/trunk: include/loc/_messages.h src/messages.cpp
>>
>> faridz@apache.org wrote:
>>> Author: faridz
>>> Date: Tue Sep 18 10:56:44 2007
>>> New Revision: 577000
>> I'm curious about this change...
>>
[...]
> Without that patch the 22.locale.stdcxx-554.cpp regression test
> failed to link on ICC, but succeeded on MSVC.
[...]
> I'm surprised, that ICC does that because that functions
> (messages<>::open()
> and messages<>::close()) are not invoked in that test, but ICC put them
> into
> .obj file even in optimized mode (12d).
How about this: I'd like us to outline the functions regardless
of these errors because I believe it's an improvement (I've done
it for most other facets and classes in the library so we have
a good precedent :)
I've outlined them here:
http://svn.apache.org/viewvc?rev=577098&view=rev
Could you try, as an experiment, to revert the dllexport change
to see if the linker errors go away? The reason for doing that
is that the fewer symbols we export from the library the less
of an ABI footprint we have to worry about maintaining, and the
easier it will be to maintain and test binary compatibility.
Martin
RE: svn commit: r577000 - in /incubator/stdcxx/trunk: include/loc/_messages.h src/messages.cpp
Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Martin Sebor [mailto:sebor@roguewave.com]
> Sent: Tuesday, September 18, 2007 10:24 PM
> To: stdcxx-dev@incubator.apache.org
> Subject: Re: svn commit: r577000 - in
> /incubator/stdcxx/trunk: include/loc/_messages.h src/messages.cpp
>
> faridz@apache.org wrote:
> > Author: faridz
> > Date: Tue Sep 18 10:56:44 2007
> > New Revision: 577000
>
> I'm curious about this change...
>
> I thought dllexport was only necessary with symbols
> (functions or objects) that could be referenced from user
> code (either directly, or indirectly via our own inline
> functions), and that references to our own symbols from with
> the stdcxx library itself, even from other translation units,
> didn't need the decoration.
>
> In this case, I note that __rw_cat_open() is being called
> from a function that's declared inline (messages::do_open()
> in the primary template), so at the first glance the change
> makes sense.
> But then I also note that we're explicitly instantiating the
> primary template on char and wchar_t (no other
> specializations are allowed), which should prevent the
> do_open() member function from being implicitly instantiated
> in code that uses it, and that the dllexport shouldn't be necessary.
>
> So, does this mean that I'm missing something or that
> something isn't working quite the way it should? Could it be
> that MSVC is actually inlining the virtual do_open()? (Most
> other compilers don't inline virtual function so I would tend
> to define it out of line to save the little bit of space and
> avoid code for it from being emitted in every object file
> that calls it).
Without that patch the 22.locale.stdcxx-554.cpp regression test
failed to link on ICC, but succeeded on MSVC.
http://people.apache.org/~sebor/stdcxx/results/win_2003-1-em64t-icl-10.0
-12D-win32-576688-log.gz.txt
http://people.apache.org/~sebor/stdcxx/results/win_2003-1-em64t-icl-10.0
-12d-win32-576688-log.gz.txt
http://people.apache.org/~sebor/stdcxx/results/win_2003-1-x86-icl-9.1-12
d-win32-576688-log.gz.txt
22.locale.stdcxx-554.cpp
Linking... (Intel C++ Environment)
xilink: executing 'link'
22.locale.stdcxx-554.obj : error LNK2019: unresolved external symbol
"int __cdecl __rw::__rw_cat_open(class std::basic_string<char,struct
std::char_traits<char>,class std::allocator<char> > const &,struct
std::locale const &)"
(?__rw_cat_open@__rw@@YAHABV?$basic_string@DU?$char_traits@D@std@@V?$all
ocator@D@2@@std@@ABUlocale@3@@Z) referenced in function "protected:
virtual int __thiscall std::messages<char>::do_open(class
std::basic_string<char,struct std::char_traits<char>,class
std::allocator<char> > const &,struct std::locale const &)const "
(?do_open@?$messages@D@std@@MBEHABV?$basic_string@DU?$char_traits@D@std@
@V?$allocator@D@2@@2@ABUlocale@2@@Z)
22.locale.stdcxx-554.obj : error LNK2019: unresolved external symbol
"void __cdecl __rw::__rw_cat_close(int)" (?__rw_cat_close@__rw@@YAXH@Z)
referenced in function "protected: virtual void __thiscall
std::messages<char>::do_close(int)const "
(?do_close@?$messages@D@std@@MBEXH@Z)
$(BUILDDIR)\12d\tests\22.locale.stdcxx-554.exe : fatal error LNK1120: 2
unresolved externals
I'm surprised, that ICC does that because that functions
(messages<>::open()
and messages<>::close()) are not invoked in that test, but ICC put them
into
.obj file even in optimized mode (12d).
Farid.
Re: svn commit: r577000 - in /incubator/stdcxx/trunk: include/loc/_messages.h
src/messages.cpp
Posted by Martin Sebor <se...@roguewave.com>.
faridz@apache.org wrote:
> Author: faridz
> Date: Tue Sep 18 10:56:44 2007
> New Revision: 577000
I'm curious about this change...
I thought dllexport was only necessary with symbols (functions
or objects) that could be referenced from user code (either
directly, or indirectly via our own inline functions), and that
references to our own symbols from with the stdcxx library itself,
even from other translation units, didn't need the decoration.
In this case, I note that __rw_cat_open() is being called from
a function that's declared inline (messages::do_open() in the
primary template), so at the first glance the change makes sense.
But then I also note that we're explicitly instantiating the
primary template on char and wchar_t (no other specializations
are allowed), which should prevent the do_open() member function
from being implicitly instantiated in code that uses it, and
that the dllexport shouldn't be necessary.
So, does this mean that I'm missing something or that something
isn't working quite the way it should? Could it be that MSVC is
actually inlining the virtual do_open()? (Most other compilers
don't inline virtual function so I would tend to define it out
of line to save the little bit of space and avoid code for it
from being emitted in every object file that calls it).
Martin
>
> URL: http://svn.apache.org/viewvc?rev=577000&view=rev
> Log:
> 2007-09-18 Farid Zaripov <Fa...@epam.com>
>
> * _messages.h (__rw_cat_open): Declare fnuction as _RWSTD_EXPORT.
> * _messages.h (__rw_get_message): Ditto.
> * _messages.h (__rw_get_locale): Ditto.
> * _messages.h (__rw_cat_close): Ditto.
> * messages.cpp (__rw_cat_open): Ditto.
> * messages.cpp (__rw_get_message): Ditto.
> * messages.cpp (__rw_get_locale): Ditto.
> * messages.cpp (__rw_cat_close): Ditto.
>
> Modified:
> incubator/stdcxx/trunk/include/loc/_messages.h
> incubator/stdcxx/trunk/src/messages.cpp
>
> Modified: incubator/stdcxx/trunk/include/loc/_messages.h
> URL: http://svn.apache.org/viewvc/incubator/stdcxx/trunk/include/loc/_messages.h?rev=577000&r1=576999&r2=577000&view=diff
> ==============================================================================
> --- incubator/stdcxx/trunk/include/loc/_messages.h (original)
> +++ incubator/stdcxx/trunk/include/loc/_messages.h Tue Sep 18 10:56:44 2007
> @@ -49,13 +49,17 @@
>
> _RWSTD_NAMESPACE (__rw) {
>
> -int __rw_cat_open (const _STD::string&, const _V3_LOCALE::locale&);
> +_RWSTD_EXPORT int
> +__rw_cat_open (const _STD::string&, const _V3_LOCALE::locale&);
>
> -const char* __rw_get_message (int, int, int);
> +_RWSTD_EXPORT const char*
> +__rw_get_message (int, int, int);
>
> -const _V3_LOCALE::locale& __rw_get_locale (int);
> +_RWSTD_EXPORT const _V3_LOCALE::locale&
> +__rw_get_locale (int);
>
> -void __rw_cat_close (int);
> +_RWSTD_EXPORT void
> +__rw_cat_close (int);
>
> } // namespace __rw
>
>
> Modified: incubator/stdcxx/trunk/src/messages.cpp
> URL: http://svn.apache.org/viewvc/incubator/stdcxx/trunk/src/messages.cpp?rev=577000&r1=576999&r2=577000&view=diff
> ==============================================================================
> --- incubator/stdcxx/trunk/src/messages.cpp (original)
> +++ incubator/stdcxx/trunk/src/messages.cpp Tue Sep 18 10:56:44 2007
> @@ -210,7 +210,7 @@
>
> // Open a message catalog and assign and return a handle for it.
>
> -int
> +_RWSTD_EXPORT int
> __rw_cat_open (const _STD::string &cat_name, const _STD::locale &loc)
> {
> _RWSTD_MT_CLASS_GUARD (__rw_open_cat_data);
> @@ -233,7 +233,7 @@
>
>
> // Get message text from catalog.
> -const char*
> +_RWSTD_EXPORT const char*
> __rw_get_message (int cat, int set_num, int msg_num)
> {
> if (cat < 0)
> @@ -261,7 +261,7 @@
>
>
> // Get locale to be used for character translation for this message catalog.
> -const _STD::locale&
> +_RWSTD_EXPORT const _STD::locale&
> __rw_get_locale (int cat)
> {
> _RWSTD_MT_CLASS_GUARD (__rw_open_cat_data);
> @@ -276,7 +276,7 @@
>
>
> // Close a catalog and release its handle.
> -void
> +_RWSTD_EXPORT void
> __rw_cat_close (int cat)
> {
> _RWSTD_MT_CLASS_GUARD (__rw_open_cat_data);
>
>