Libstdc++ as DLL on windows, alternative approach [was Re: cygwin patch review]

[Danny Smith]

On Sat, Nov 21, 2009 at 5:59 AM, Dave Korn  wrote:
> Benjamin Kosnik wrote:
>
>>> However it does sound like the right thing to do, so I've spun a new
>>> version of the patch that applies dllimport to the namespace.  We
>>> could commit this and then open a PR about dllimport not working on
>>> namespaces.
>
>> I like this plan. What's the PR number? And can you affix your
>> patch from here to it:
>> http://gcc.gnu.org/ml/gcc-patches/2009-11/msg00979.html
>
>  This is now PR 42123.  (The WIP patch is on the right lines but not quite
> correct yet, details in the PR.)
>
>> Thanks.
>>
>>>   Initial test results (c and c++ only) are posted(*) and look right,
>>> but I'll do a clean run on the same source revision for comparison
>>> anyway.  Note that I had to patch various testsuite/lib/ files to
>>> avoid the current utf-8 problem(**) that would otherwise have
>>> provoked many spurious 'excess errors' warnings.  That'll be the
>>> subject of a separate patch, I'll do the clean run using them as well
>>> so the comparison is fair.
>>
>> It looks like (in later messages) you have an updated patch.
>
>  I've since made one small change (apart from experimenting with the
> namespace patch).  In testing the namespace patch, I discovered an order of
> include problem, where this code from include/bits/c++config:
>
>>  #if _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY
>>  # define _GLIBCXX_VISIBILITY_ATTR(V) __attribute__ ((__visibility__ (#V)))
>> +#elif defined (_GLIBCXX_PSEUDO_VISIBILITY)
>> +# define _GLIBCXX_VISIBILITY_ATTR(V) _GLIBCXX_PSEUDO_VISIBILITY(V)
>>  #else
>>  # define _GLIBCXX_VISIBILITY_ATTR(V)
>>  #endif
>
> is before any of the os/cpu-specific headers get #included, so it didn't work.
>  I've rearranged it now so that this site at the top of the file just says
>
>> #if _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY
>> # define _GLIBCXX_VISIBILITY_ATTR(V) __attribute__ ((__visibility__ (#V)))
>> #else
>> // If this is not supplied by the OS-specific or CPU-specific
>> // headers included below, it will be defined to an empty default.
>> # define _GLIBCXX_VISIBILITY_ATTR(V) _GLIBCXX_PSEUDO_VISIBILITY(V)
>> #endif
>
> .. and then after the #includes I added
>
>> // If platform uses neither visibility nor psuedo-visibility,
>> // specify empty default for namespace annotation macros.
>> #ifndef _GLIBCXX_PSEUDO_VISIBILITY
>> #define _GLIBCXX_PSEUDO_VISIBILITY(V)
>> #endif
>
>  I've verified this change DTRT by diffing pre-processed sources compiled
> with and without -D_GLIBCXX_DLL.
>
>> But your
>> updated results here looks great:
>> http://gcc.gnu.org/ml/gcc-testresults/2009-11/msg01084.html
>
>  Yep, the results are now consistently the same as for static libstdc++.
>
>>>   So, is this any more like OK for head?
>>
>> Yes, exactly. Thanks. This is OK with me.
>
>  :)  Fantastic.  Three small things which need explicit confirmation:
>
> - can your OK cover the c++-related change to LINK_SPEC in gcc/gcc.c or do I
> need to get another maintainer (probably jsm) for that?
> - is it OK even with the change to the c++config hunk mentioned above?
> - is it OK even in stage 3?
>
>  Also, PING! Danny/Kai, the windows side of this patch has changed a bit
> since you guys OK'd it some time back, can one of you take a look over the new
> changes to config/i386/winnt.c w.r.t. i386_find_on_wrapper_list() and #ifdef
> CXX_WRAP_SPEC_LIST?
>
>  I've just updated my sandbox to head.  Attached, the final version of the
> patch for reference; I'm just rebuilding it now to check nothing has broken.
> The ChangeLog is almost the same as last time, but mentions the c++config change:
>

OK. Thanks

Danny