Patchwork --enable-dynamic-string default for mingw-w64 v2

login
register
mail settings
Submitter JonY
Date Oct. 1, 2011, 6:03 a.m.
Message ID <4E86AD37.9020407@users.sourceforge.net>
Download mbox | patch
Permalink /patch/117257/
State New
Headers show

Comments

JonY - Oct. 1, 2011, 6:03 a.m.
Hi,

I followed Paolo's suggestion with the os_defines.h trick. I duplicated
os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any
built-in defines to tell the 2 apart unless you include some headers
like _mingw.h.

Patch attached, comments?
Ozkan Sezer - Oct. 1, 2011, 6:29 a.m.
On Sat, Oct 1, 2011 at 9:03 AM, JonY <jon_y@users.sourceforge.net> wrote:
> Hi,
>
> I followed Paolo's suggestion with the os_defines.h trick. I duplicated
> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any
> built-in defines to tell the 2 apart unless you include some headers
> like _mingw.h.
>
> Patch attached, comments?

Why _GLIBCXX_FULLY_DYNAMIC_STRING being defined or not defined
is not enough?

--
O.S.
Paolo Carlini - Oct. 1, 2011, 9:15 a.m.
On 10/01/2011 08:03 AM, JonY wrote:
> Hi,
>
> I followed Paolo's suggestion with the os_defines.h trick. I duplicated
> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any
> built-in defines to tell the 2 apart unless you include some headers
> like _mingw.h.
>
> Patch attached, comments?
These:

+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)


can be simplified to just:

+#if _GLIBCXX_FULLY_DYNAMIC_STRING == 0


Ok with those changes and a proper ChangeLog entry, of course.

Thanks,
Paolo.
Pedro Alves - Oct. 1, 2011, 10:33 a.m.
On Saturday 01 October 2011 07:03:35, JonY wrote:
> Hi,
> 
> I followed Paolo's suggestion with the os_defines.h trick. I duplicated
> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any
> built-in defines to tell the 2 apart unless you include some headers
> like _mingw.h.

Are we really introducing a bunch of duplication between 
os/mingw32/ and os/mingw32-w64/ ?  I didn't see the part that adds the
new dir and does all those copies in the patch; where is it?  Or have
I missed something?  Can't we make configure add
-D__IM_REALLY_W64_YOU_KNOW to CFLAGS instead?  Or come up with a way
to point libstd++ to pick up a new mingw32/os_defines_w64.h file instead
that does:

#include "os_defines.h"
// mingw-w64 should use fully-dynamic-string by default
#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
#define _GLIBCXX_FULLY_DYNAMIC_STRING 1
#endif
JonY - Oct. 1, 2011, 11:15 a.m.
On 10/1/2011 18:33, Pedro Alves wrote:
> On Saturday 01 October 2011 07:03:35, JonY wrote:
>> Hi,
>>
>> I followed Paolo's suggestion with the os_defines.h trick. I duplicated
>> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any
>> built-in defines to tell the 2 apart unless you include some headers
>> like _mingw.h.
> 
> Are we really introducing a bunch of duplication between 
> os/mingw32/ and os/mingw32-w64/ ?  I didn't see the part that adds the
> new dir and does all those copies in the patch; where is it?  Or have
> I missed something?  Can't we make configure add
> -D__IM_REALLY_W64_YOU_KNOW to CFLAGS instead?  Or come up with a way
> to point libstd++ to pick up a new mingw32/os_defines_w64.h file instead
> that does:
> 
> #include "os_defines.h"
> // mingw-w64 should use fully-dynamic-string by default
> #ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
> #define _GLIBCXX_FULLY_DYNAMIC_STRING 1
> #endif
> 

The new files are missing from svn diff because I used svn copy to copy
the directories, svn diff didn't show them, should I use something else
instead?

IMHO, mingw.org and mingw-w64 may or may not diverge further in the
future, so sprinkling defines to codes isn't very good in the long run.
Pedro Alves - Oct. 1, 2011, 1:01 p.m.
On Saturday 01 October 2011 12:15:42, JonY wrote:
> On 10/1/2011 18:33, Pedro Alves wrote:
> > On Saturday 01 October 2011 07:03:35, JonY wrote:
> >> Hi,
> >>
> >> I followed Paolo's suggestion with the os_defines.h trick. I duplicated
> >> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any
> >> built-in defines to tell the 2 apart unless you include some headers
> >> like _mingw.h.
> > 
> > Are we really introducing a bunch of duplication between 
> > os/mingw32/ and os/mingw32-w64/ ?  I didn't see the part that adds the
> > new dir and does all those copies in the patch; where is it?  Or have
> > I missed something?  Can't we make configure add
> > -D__IM_REALLY_W64_YOU_KNOW to CFLAGS instead?  Or come up with a way
> > to point libstd++ to pick up a new mingw32/os_defines_w64.h file instead
> > that does:
> > 
> > #include "os_defines.h"
> > // mingw-w64 should use fully-dynamic-string by default
> > #ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
> > #define _GLIBCXX_FULLY_DYNAMIC_STRING 1
> > #endif
> > 
> 
> The new files are missing from svn diff because I used svn copy to copy
> the directories, svn diff didn't show them, should I use something else
> instead?

So that'd be a patch with its own ChangeLog, as your patch applies on
top of that already.

> IMHO, mingw.org and mingw-w64 may or may not diverge further in the
> future, so sprinkling defines to codes isn't very good in the long run.

"may or may not" is key.  We don't know the future, but we know
the present. We do know that code duplication is bad.  I can just
as well say,

IMHO, mingw.org and mingw-w64 may or may not diverge further in the
future (ideally they wouldn't), so adding code duplication when
we only need one define isn't very good in the long run.

But I'm not a maintainer, so I shall just go away.
Kai Tietz - Oct. 1, 2011, 3:06 p.m.
2011/10/1 Pedro Alves <pedro@codesourcery.com>:
> On Saturday 01 October 2011 12:15:42, JonY wrote:
>> On 10/1/2011 18:33, Pedro Alves wrote:
>> > On Saturday 01 October 2011 07:03:35, JonY wrote:
>> >> Hi,
>> >>
>> >> I followed Paolo's suggestion with the os_defines.h trick. I duplicated
>> >> os/mingw32/ to os/mingw32-w64/ for this to work, since there aren't any
>> >> built-in defines to tell the 2 apart unless you include some headers
>> >> like _mingw.h.
>> >
>> > Are we really introducing a bunch of duplication between
>> > os/mingw32/ and os/mingw32-w64/ ?  I didn't see the part that adds the
>> > new dir and does all those copies in the patch; where is it?  Or have
>> > I missed something?  Can't we make configure add
>> > -D__IM_REALLY_W64_YOU_KNOW to CFLAGS instead?  Or come up with a way
>> > to point libstd++ to pick up a new mingw32/os_defines_w64.h file instead
>> > that does:
>> >
>> > #include "os_defines.h"
>> > // mingw-w64 should use fully-dynamic-string by default
>> > #ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
>> > #define _GLIBCXX_FULLY_DYNAMIC_STRING 1
>> > #endif
>> >
>>
>> The new files are missing from svn diff because I used svn copy to copy
>> the directories, svn diff didn't show them, should I use something else
>> instead?
>
> So that'd be a patch with its own ChangeLog, as your patch applies on
> top of that already.
>
>> IMHO, mingw.org and mingw-w64 may or may not diverge further in the
>> future, so sprinkling defines to codes isn't very good in the long run.
>
> "may or may not" is key.  We don't know the future, but we know
> the present. We do know that code duplication is bad.  I can just
> as well say,

Well, this situation isn't ideal.  It might be that one day mingw.org
and mingw-w64 venture come more near in feature-set.  But this is for
sure more a long-term issue and not a short or medium-term thing.

> IMHO, mingw.org and mingw-w64 may or may not diverge further in the
> future (ideally they wouldn't), so adding code duplication when
> we only need one define isn't very good in the long run.
>
> But I'm not a maintainer, so I shall just go away.

Well, we diverge already more and more.  I plan to provide for
libstdc++ some new printf/scanf API features for wide and ascii
variants, which is present in mingw-w64 venture, but not in mingw.org
venture.  We have also other divergencies in other feature-set, which
lead already to an add-on header in gcc for specific mingw-w64
targets.

Kai

Patch

Index: configure.host
===================================================================
--- configure.host	(revision 179411)
+++ configure.host	(working copy)
@@ -260,8 +260,15 @@ 
     atomic_word_dir=os/irix
     ;;
   mingw32*)
-    os_include_dir="os/mingw32"
-    error_constants_dir="os/mingw32"
+    case "$host" in
+      *-w64-*)
+        os_include_dir="os/mingw32-w64"
+        error_constants_dir="os/mingw32-w64"
+        ;;
+      *)
+        os_include_dir="os/mingw32"
+        error_constants_dir="os/mingw32"
+        ;;
     OPT_LDFLAGS="${OPT_LDFLAGS} \$(lt_host_flags)"
     ;;
   netbsd*)
Index: include/bits/basic_string.h
===================================================================
--- include/bits/basic_string.h	(revision 179411)
+++ include/bits/basic_string.h	(working copy)
@@ -201,7 +201,7 @@ 
 	void
 	_M_set_length_and_sharable(size_type __n)
 	{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
 	  if (__builtin_expect(this != &_S_empty_rep(), false))
 #endif
 	    {
@@ -231,7 +231,7 @@ 
 	void
 	_M_dispose(const _Alloc& __a)
 	{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
 	  if (__builtin_expect(this != &_S_empty_rep(), false))
 #endif
 	    {
@@ -252,7 +252,7 @@ 
 	_CharT*
 	_M_refcopy() throw()
 	{
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
 	  if (__builtin_expect(this != &_S_empty_rep(), false))
 #endif
             __gnu_cxx::__atomic_add_dispatch(&this->_M_refcount, 1);
@@ -430,7 +430,7 @@ 
        *  @brief  Default constructor creates an empty string.
        */
       basic_string()
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
       : _M_dataplus(_S_empty_rep()._M_refdata(), _Alloc()) { }
 #else
       : _M_dataplus(_S_construct(size_type(), _CharT(), _Alloc()), _Alloc()){ }
@@ -502,7 +502,7 @@ 
       basic_string(basic_string&& __str) noexcept
       : _M_dataplus(__str._M_dataplus)
       {
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING	
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
 	__str._M_data(_S_empty_rep()._M_refdata());
 #else
 	__str._M_data(_S_construct(size_type(), _CharT(), get_allocator()));
Index: include/bits/basic_string.tcc
===================================================================
--- include/bits/basic_string.tcc	(revision 179411)
+++ include/bits/basic_string.tcc	(working copy)
@@ -80,7 +80,7 @@ 
       _S_construct(_InIterator __beg, _InIterator __end, const _Alloc& __a,
 		   input_iterator_tag)
       {
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
 	if (__beg == __end && __a == _Alloc())
 	  return _S_empty_rep()._M_refdata();
 #endif
@@ -126,7 +126,7 @@ 
       _S_construct(_InIterator __beg, _InIterator __end, const _Alloc& __a,
 		   forward_iterator_tag)
       {
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
 	if (__beg == __end && __a == _Alloc())
 	  return _S_empty_rep()._M_refdata();
 #endif
@@ -154,7 +154,7 @@ 
     basic_string<_CharT, _Traits, _Alloc>::
     _S_construct(size_type __n, _CharT __c, const _Alloc& __a)
     {
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
       if (__n == 0 && __a == _Alloc())
 	return _S_empty_rep()._M_refdata();
 #endif
@@ -456,7 +456,7 @@ 
     basic_string<_CharT, _Traits, _Alloc>::
     _M_leak_hard()
     {
-#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#if !defined (_GLIBCXX_FULLY_DYNAMIC_STRING) || (_GLIBCXX_FULLY_DYNAMIC_STRING == 0)
       if (_M_rep() == &_S_empty_rep())
 	return;
 #endif
Index: config/os/mingw32-w64/os_defines.h
===================================================================
--- config/os/mingw32-w64/os_defines.h	(revision 179411)
+++ config/os/mingw32-w64/os_defines.h	(working copy)
@@ -65,4 +65,9 @@ 
 // ioctlsocket function doesn't work for normal file-descriptors.
 #define _GLIBCXX_NO_IOCTL 1
 
+// mingw-w64 should use fully-dynamic-string by default
+#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
+#define _GLIBCXX_FULLY_DYNAMIC_STRING 1
 #endif
+
+#endif
Index: acinclude.m4
===================================================================
--- acinclude.m4	(revision 179411)
+++ acinclude.m4	(working copy)
@@ -564,17 +564,21 @@ 
 dnl memory (mostly useful together with shared memory allocators, see PR
 dnl libstdc++/16612 for details).
 dnl
-dnl --enable-fully-dynamic-string defines _GLIBCXX_FULLY_DYNAMIC_STRING
-dnl --disable-fully-dynamic-string leaves _GLIBCXX_FULLY_DYNAMIC_STRING undefined
+dnl --enable-fully-dynamic-string defines _GLIBCXX_FULLY_DYNAMIC_STRING to 1
+dnl --disable-fully-dynamic-string defines _GLIBCXX_FULLY_DYNAMIC_STRING to 0
+dnl otherwise undefined
 dnl  +  Usage:  GLIBCXX_ENABLE_FULLY_DYNAMIC_STRING[(DEFAULT)]
 dnl       Where DEFAULT is either `yes' or `no'.
 dnl
 AC_DEFUN([GLIBCXX_ENABLE_FULLY_DYNAMIC_STRING], [
   GLIBCXX_ENABLE(fully-dynamic-string,$1,,[do not put empty strings in per-process static memory])
   if test $enable_fully_dynamic_string = yes; then
-    AC_DEFINE(_GLIBCXX_FULLY_DYNAMIC_STRING, 1,
-	      [Define if a fully dynamic basic_string is wanted.])
+    enable_fully_dynamic_string_def=1
+  else
+    enable_fully_dynamic_string_def=0
   fi
+  AC_DEFINE(_GLIBCXX_FULLY_DYNAMIC_STRING, $enable_fully_dynamic_string_def,
+	      [Define to 1 if a fully dynamic basic_string is wanted, 0 to disable, undefined for platform defaults])
 ])