| Message ID | 4E86AD37.9020407@users.sourceforge.net |
|---|---|
| State | New |
| Headers | show |
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.
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.
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
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.
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.
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
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]) ])