From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13951 invoked by alias); 21 Nov 2009 06:31:35 -0000 Received: (qmail 13934 invoked by uid 22791); 21 Nov 2009 06:31:34 -0000 X-SWARE-Spam-Status: No, hits=-0.5 required=5.0 tests=AWL,BAYES_50,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-yw0-f180.google.com (HELO mail-yw0-f180.google.com) (209.85.211.180) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 21 Nov 2009 06:30:43 +0000 Received: by ywh10 with SMTP id 10so3657640ywh.8 for ; Fri, 20 Nov 2009 22:30:41 -0800 (PST) MIME-Version: 1.0 Received: by 10.90.58.2 with SMTP id g2mr3735084aga.73.1258785041680; Fri, 20 Nov 2009 22:30:41 -0800 (PST) In-Reply-To: <4B06CAEA.5080705@gmail.com> References: <20091019092452.2e271791@mcgee.artheist.org> <4AFC8EC3.10502@gmail.com> <20091119093347.797df3ff@mcgee.artheist.org> <4B06CAEA.5080705@gmail.com> Date: Sat, 21 Nov 2009 06:34:00 -0000 Message-ID: <9c03c2dd0911202230x13f5ab00l6033ba648ecf5e6b@mail.gmail.com> Subject: Re: Libstdc++ as DLL on windows, alternative approach [was Re: cygwin patch review] From: Danny Smith To: Dave Korn Cc: Benjamin Kosnik , libstdc++@gcc.gnu.org, GCC Patches , "Aaron W. LaFramboise (GCC)" , Kai Tietz Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2009-11/txt/msg01146.txt.bz2 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. =A0We >>> 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 > > =A0This is now PR 42123. =A0(The WIP patch is on the right lines but not = quite > correct yet, details in the PR.) > >> Thanks. >> >>> =A0 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. =A0Note 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. =A0That'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. > > =A0I've since made one small change (apart from experimenting with the > namespace patch). =A0In testing the namespace patch, I discovered an orde= r of > include problem, where this code from include/bits/c++config: > >> =A0#if _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY >> =A0# define _GLIBCXX_VISIBILITY_ATTR(V) __attribute__ ((__visibility__ (= #V))) >> +#elif defined (_GLIBCXX_PSEUDO_VISIBILITY) >> +# define _GLIBCXX_VISIBILITY_ATTR(V) _GLIBCXX_PSEUDO_VISIBILITY(V) >> =A0#else >> =A0# define _GLIBCXX_VISIBILITY_ATTR(V) >> =A0#endif > > is before any of the os/cpu-specific headers get #included, so it didn't = work. > =A0I've rearranged it now so that this site at the top of the file just s= ays > >> #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 > > =A0I've verified this change DTRT by diffing pre-processed sources compil= ed > with and without -D_GLIBCXX_DLL. > >> But your >> updated results here looks great: >> http://gcc.gnu.org/ml/gcc-testresults/2009-11/msg01084.html > > =A0Yep, the results are now consistently the same as for static libstdc++. > >>> =A0 So, is this any more like OK for head? >> >> Yes, exactly. Thanks. This is OK with me. > > =A0:) =A0Fantastic. =A0Three small things which need explicit confirmatio= n: > > - can your OK cover the c++-related change to LINK_SPEC in gcc/gcc.c or d= o 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? > > =A0Also, 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 th= e new > changes to config/i386/winnt.c w.r.t. i386_find_on_wrapper_list() and #if= def > CXX_WRAP_SPEC_LIST? > > =A0I've just updated my sandbox to head. =A0Attached, the final version o= f the > patch for reference; I'm just rebuilding it now to check nothing has brok= en. > The ChangeLog is almost the same as last time, but mentions the c++config= change: > OK. Thanks Danny