Sorry this should have gone to the list... On 6/17/2011 10:05 AM, Jim Bell wrote:
On 6/17/2011 9:37 AM, Rene Rivera wrote:
On 6/17/2011 8:58 AM, Jim Bell wrote:
On 1:59 PM, Rene Rivera wrote:
The release branch is now close, i.e. the release is frozen, for further changes. This means that commits to the release branch will fail without explicit authorization.
The Boost.Thread patch for Mingw-64 is crucial for that important platform.<https://svn.boost.org/trac/boost/ticket/4849>
It has been patched into the trunk for three months without complaint, and just needs to be shepherded to the release branch.
Could you please do this?
I'm not sure what you are asking.
I'd like a maintainer or release manager to merge the patched trunk files into the release branch. boost/detail/interlocked.hpp libs/thread/src/win32/thread.cpp libs/thread/src/win32/tss_pe.cpp
Since I don't know enough to judge the code itself at this instant.. Could others please comment on the above? Is the thread maintainer around? -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail
On 6/17/2011 10:11 AM, Rene Rivera wrote:
Sorry this should have gone to the list...
On 6/17/2011 10:05 AM, Jim Bell wrote:
On 6/17/2011 9:37 AM, Rene Rivera wrote:
On 6/17/2011 8:58 AM, Jim Bell wrote:
On 1:59 PM, Rene Rivera wrote:
The release branch is now close, i.e. the release is frozen, for further changes. This means that commits to the release branch will fail without explicit authorization.
The Boost.Thread patch for Mingw-64 is crucial for that important platform.<https://svn.boost.org/trac/boost/ticket/4849>
It has been patched into the trunk for three months without complaint, and just needs to be shepherded to the release branch.
Could you please do this?
I'm not sure what you are asking.
I'd like a maintainer or release manager to merge the patched trunk files into the release branch. boost/detail/interlocked.hpp libs/thread/src/win32/thread.cpp libs/thread/src/win32/tss_pe.cpp
Since I don't know enough to judge the code itself at this instant.. Could others please comment on the above? Is the thread maintainer around?
Actually, I looked at the changeset.. And I don't see how that change is safe. I can see one immediate compile error/problem, 7 likely other compile problems, and the high-risk of changing a header in a way that impacts 3 platforms (and their related numerous compilers). So.. No. I won't merge that, or want any one else to merge that at this point. -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail
Rene Rivera <grafikrobot@gmail.com> writes:
On 6/17/2011 10:05 AM, Jim Bell wrote:
On 6/17/2011 9:37 AM, Rene Rivera wrote:
On 6/17/2011 8:58 AM, Jim Bell wrote:
On 1:59 PM, Rene Rivera wrote:
The release branch is now close, i.e. the release is frozen, for further changes. This means that commits to the release branch will fail without explicit authorization.
The Boost.Thread patch for Mingw-64 is crucial for that important platform.<https://svn.boost.org/trac/boost/ticket/4849>
It has been patched into the trunk for three months without complaint, and just needs to be shepherded to the release branch.
Could you please do this?
I'm not sure what you are asking.
I'd like a maintainer or release manager to merge the patched trunk files into the release branch. boost/detail/interlocked.hpp libs/thread/src/win32/thread.cpp libs/thread/src/win32/tss_pe.cpp
Since I don't know enough to judge the code itself at this instant.. Could others please comment on the above? Is the thread maintainer around?
The only change NOT already merged is the change to boost/detail/interlocked.hpp. I overlooked this when I merged the thread changes from trunk to release a couple of weeks ago, and I wouldn't have remembered if Jim hadn't raised the issue, since this is not one of the main thread headers. However, this is a relatively small change (some functions are no longer marked dllimport), and it only affects this one compiler, since the code expands to the same as before on other compilers. Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
On 6/17/2011 10:36 AM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
On 6/17/2011 10:05 AM, Jim Bell wrote:
On 6/17/2011 9:37 AM, Rene Rivera wrote:
On 6/17/2011 8:58 AM, Jim Bell wrote:
On 1:59 PM, Rene Rivera wrote:
The release branch is now close, i.e. the release is frozen, for further changes. This means that commits to the release branch will fail without explicit authorization.
The Boost.Thread patch for Mingw-64 is crucial for that important platform.<https://svn.boost.org/trac/boost/ticket/4849>
It has been patched into the trunk for three months without complaint, and just needs to be shepherded to the release branch.
Could you please do this?
I'm not sure what you are asking.
I'd like a maintainer or release manager to merge the patched trunk files into the release branch. boost/detail/interlocked.hpp libs/thread/src/win32/thread.cpp libs/thread/src/win32/tss_pe.cpp
Since I don't know enough to judge the code itself at this instant.. Could others please comment on the above? Is the thread maintainer around?
The only change NOT already merged is the change to boost/detail/interlocked.hpp. I overlooked this when I merged the thread changes from trunk to release a couple of weeks ago, and I wouldn't have remembered if Jim hadn't raised the issue, since this is not one of the main thread headers.
However, this is a relatively small change (some functions are no longer marked dllimport), and it only affects this one compiler, since the code expands to the same as before on other compilers.
Could you address my other concerns I posted about? -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail
Rene Rivera <grafikrobot@gmail.com> writes:
On 6/17/2011 10:36 AM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
On 6/17/2011 10:05 AM, Jim Bell wrote:
On 6/17/2011 9:37 AM, Rene Rivera wrote:
On 6/17/2011 8:58 AM, Jim Bell wrote:
On 1:59 PM, Rene Rivera wrote: The Boost.Thread patch for Mingw-64 is crucial for that important platform.<https://svn.boost.org/trac/boost/ticket/4849>
It has been patched into the trunk for three months without complaint, and just needs to be shepherded to the release branch.
I'd like a maintainer or release manager to merge the patched trunk files into the release branch. boost/detail/interlocked.hpp libs/thread/src/win32/thread.cpp libs/thread/src/win32/tss_pe.cpp
Since I don't know enough to judge the code itself at this instant.. Could others please comment on the above? Is the thread maintainer around?
The only change NOT already merged is the change to boost/detail/interlocked.hpp. I overlooked this when I merged the thread changes from trunk to release a couple of weeks ago, and I wouldn't have remembered if Jim hadn't raised the issue, since this is not one of the main thread headers.
However, this is a relatively small change (some functions are no longer marked dllimport), and it only affects this one compiler, since the code expands to the same as before on other compilers.
Could you address my other concerns I posted about?
I presume you mean:
Actually, I looked at the changeset.. And I don't see how that change is safe. I can see one immediate compile error/problem, 7 likely other compile problems, and the high-risk of changing a header in a way that impacts 3 platforms (and their related numerous compilers).
I don't see the "immediate compile error/problem" or "7 likely other compile problems" that you see. As Jim points out, this has been on trunk for 3 months, without any apparent problems. If I'd remembered, I would have merged it when I merged the other thread changes. I think the change is safe. As Jim points out, boost.thread will be unusable on mingw64 without it. If it were my choice, I'd merge it, but it's your call, not mine. Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
On 6/17/2011 11:24 AM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
On 6/17/2011 10:36 AM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
On 6/17/2011 10:05 AM, Jim Bell wrote:
On 6/17/2011 9:37 AM, Rene Rivera wrote:
On 6/17/2011 8:58 AM, Jim Bell wrote: > > On 1:59 PM, Rene Rivera wrote: > The Boost.Thread patch for Mingw-64 is crucial for that important > platform.<https://svn.boost.org/trac/boost/ticket/4849> > > It has been patched into the trunk for three months without complaint, > and just needs to be shepherded to the release branch. > I'd like a maintainer or release manager to merge the patched trunk files into the release branch. boost/detail/interlocked.hpp libs/thread/src/win32/thread.cpp libs/thread/src/win32/tss_pe.cpp
Since I don't know enough to judge the code itself at this instant.. Could others please comment on the above? Is the thread maintainer around?
The only change NOT already merged is the change to boost/detail/interlocked.hpp. I overlooked this when I merged the thread changes from trunk to release a couple of weeks ago, and I wouldn't have remembered if Jim hadn't raised the issue, since this is not one of the main thread headers.
However, this is a relatively small change (some functions are no longer marked dllimport), and it only affects this one compiler, since the code expands to the same as before on other compilers.
Could you address my other concerns I posted about?
I presume you mean:
Actually, I looked at the changeset.. And I don't see how that change is safe. I can see one immediate compile error/problem, 7 likely other compile problems, and the high-risk of changing a header in a way that impacts 3 platforms (and their related numerous compilers).
I don't see the "immediate compile error/problem" or "7 likely other compile problems" that you see. As Jim points out, this has been on trunk for 3 months, without any apparent problems. If I'd remembered, I would have merged it when I merged the other thread changes.
I mean this compile problem: tss_pte.cpp#14 #if #if (defined(__MINGW32__) && !defined(_WIN64)) || defined(__MINGW64__) I.e. the double #if. And, at least a potential for problems depending on which CPP hits this: extern "C"BOOST_INTERLOCKED_IMPORT long __stdcall InterlockedIncrement( long volatile * ); That is, the lack of space between "C" and BOOST_INTERLOCKED_IMPORT.
I think the change is safe. As Jim points out, boost.thread will be unusable on mingw64 without it. If it were my choice, I'd merge it, but it's your call, not mine.
-- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail
Rene Rivera <grafikrobot@gmail.com> writes:
I mean this compile problem:
tss_pte.cpp#14 #if #if (defined(__MINGW32__) && !defined(_WIN64)) || defined(__MINGW64__)
I.e. the double #if.
That's not present in the actual code (which is already merged to release anyway).
And, at least a potential for problems depending on which CPP hits this:
extern "C"BOOST_INTERLOCKED_IMPORT long __stdcall InterlockedIncrement( long volatile * );
That is, the lack of space between "C" and BOOST_INTERLOCKED_IMPORT.
OK, that's really there in the file, and this is the file that hasn't yet been merged, but it isn't causing a problem on the trunk regression tests. I don't think the whitespace is important anyway --- the grammar is: extern /string-literal/ /declaration/ Neither /string-literal/ nor /declaration/ require leading or trailing whitespace, so it should be OK (and appears to be in practice for the compilers tested on trunk). Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
On 6/17/2011 11:53 AM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
I mean this compile problem:
tss_pte.cpp#14 #if #if (defined(__MINGW32__)&& !defined(_WIN64)) || defined(__MINGW64__)
I.e. the double #if.
That's not present in the actual code (which is already merged to release anyway).
OK, so now I'm confused.. Could you please post a patch of what the change to the release branch is? From what I understand now is that neither the changeset nor the current state of threads files is telling me what the merge is.
And, at least a potential for problems depending on which CPP hits this:
extern "C"BOOST_INTERLOCKED_IMPORT long __stdcall InterlockedIncrement( long volatile * );
That is, the lack of space between "C" and BOOST_INTERLOCKED_IMPORT.
OK, that's really there in the file, and this is the file that hasn't yet been merged, but it isn't causing a problem on the trunk regression tests.
I don't think the whitespace is important anyway --- the grammar is:
extern /string-literal/ /declaration/
Neither /string-literal/ nor /declaration/ require leading or trailing whitespace, so it should be OK (and appears to be in practice for the compilers tested on trunk).
OK. -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail
Rene Rivera <grafikrobot@gmail.com> writes:
On 6/17/2011 11:53 AM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
I mean this compile problem:
tss_pte.cpp#14 #if #if (defined(__MINGW32__)&& !defined(_WIN64)) || defined(__MINGW64__)
I.e. the double #if.
That's not present in the actual code (which is already merged to release anyway).
OK, so now I'm confused.. Could you please post a patch of what the change to the release branch is? From what I understand now is that neither the changeset nor the current state of threads files is telling me what the merge is.
The only file to be merged is boost/detail/interlocked.hpp, with the following changes: anthony@lothlorien:~/boost$ diff release/boost/detail/interlocked.hpp trunk/boost/detail/interlocked.hpp 108a109,115
#if defined(__MINGW64__) #define BOOST_INTERLOCKED_IMPORT #else #define BOOST_INTERLOCKED_IMPORT __declspec(dllimport) #endif
115,119c122,131 < extern "C" __declspec(dllimport) long __stdcall InterlockedIncrement( long volatile * ); < extern "C" __declspec(dllimport) long __stdcall InterlockedDecrement( long volatile * ); < extern "C" __declspec(dllimport) long __stdcall InterlockedCompareExchange( long volatile *, long, long ); < extern "C" __declspec(dllimport) long __stdcall InterlockedExchange( long volatile *, long ); < extern "C" __declspec(dllimport) long __stdcall InterlockedExchangeAdd( long volatile *, long ); ---
extern "C"BOOST_INTERLOCKED_IMPORT long __stdcall InterlockedIncrement( long volatile * ); extern "C"BOOST_INTERLOCKED_IMPORT long __stdcall InterlockedDecrement( long volatile * ); extern "C"BOOST_INTERLOCKED_IMPORT long __stdcall InterlockedCompareExchange( long volatile *, long, long ); extern "C"BOOST_INTERLOCKED_IMPORT long __stdcall InterlockedExchange( long volatile *, long ); extern "C"BOOST_INTERLOCKED_IMPORT long __stdcall InterlockedExchangeAdd( long volatile *, long );
# if defined(_M_IA64) || defined(_M_AMD64) extern "C"BOOST_INTERLOCKED_IMPORT void* __stdcall InterlockedCompareExchangePointer( void* volatile *, void*, void* ); extern "C"BOOST_INTERLOCKED_IMPORT void* __stdcall InterlockedExchangePointer( void* volatile *, void* ); # endif 131c143,147 < # define BOOST_INTERLOCKED_COMPARE_EXCHANGE_POINTER(dest,exchange,compare) \
# if defined(_M_IA64) || defined(_M_AMD64) # define BOOST_INTERLOCKED_COMPARE_EXCHANGE_POINTER ::boost::detail::InterlockedCompareExchangePointer # define BOOST_INTERLOCKED_EXCHANGE_POINTER ::boost::detail::InterlockedExchangePointer # else # define BOOST_INTERLOCKED_COMPARE_EXCHANGE_POINTER(dest,exchange,compare) \ 133c149 < # define BOOST_INTERLOCKED_EXCHANGE_POINTER(dest,exchange) \
# define BOOST_INTERLOCKED_EXCHANGE_POINTER(dest,exchange) \ 134a151 # endif
Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
On 6/17/2011 2:17 PM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
On 6/17/2011 11:53 AM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
I mean this compile problem:
tss_pte.cpp#14 #if #if (defined(__MINGW32__)&& !defined(_WIN64)) || defined(__MINGW64__)
I.e. the double #if.
That's not present in the actual code (which is already merged to release anyway).
OK, so now I'm confused.. Could you please post a patch of what the change to the release branch is? From what I understand now is that neither the changeset nor the current state of threads files is telling me what the merge is.
The only file to be merged is boost/detail/interlocked.hpp, with the following changes:
OK, got it now. The changes seem reasonably safe now. So go ahead with a "approved by rene" commit. -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org (msn) - grafik/redshift-software.com -- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail
Rene Rivera <grafikrobot@gmail.com> writes:
On 6/17/2011 2:17 PM, Anthony Williams wrote:
The only file to be merged is boost/detail/interlocked.hpp, with the following changes:
OK, got it now. The changes seem reasonably safe now. So go ahead with a "approved by rene" commit.
Thanks. Committed as revision 72657. Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
On 1:59 PM, Rene Rivera wrote:
On 6/17/2011 10:36 AM, Anthony Williams wrote:
Rene Rivera<grafikrobot@gmail.com> writes:
On 6/17/2011 10:05 AM, Jim Bell wrote:
On 6/17/2011 9:37 AM, Rene Rivera wrote:
On 6/17/2011 8:58 AM, Jim Bell wrote:
On 1:59 PM, Rene Rivera wrote: > The release branch is now close, i.e. the release is frozen, for > further changes. This means that commits to the release branch will > fail without explicit authorization.
The Boost.Thread patch for Mingw-64 is crucial for that important platform.<https://svn.boost.org/trac/boost/ticket/4849>
It has been patched into the trunk for three months without complaint, and just needs to be shepherded to the release branch.
Could you please do this?
I'm not sure what you are asking.
I'd like a maintainer or release manager to merge the patched trunk files into the release branch. boost/detail/interlocked.hpp libs/thread/src/win32/thread.cpp libs/thread/src/win32/tss_pe.cpp
Since I don't know enough to judge the code itself at this instant.. Could others please comment on the above? Is the thread maintainer around?
The only change NOT already merged is the change to boost/detail/interlocked.hpp. I overlooked this when I merged the thread changes from trunk to release a couple of weeks ago, and I wouldn't have remembered if Jim hadn't raised the issue, since this is not one of the main thread headers.
However, this is a relatively small change (some functions are no longer marked dllimport), and it only affects this one compiler, since the code expands to the same as before on other compilers.
Could you address my other concerns I posted about?
You mentioned total concerns, but not specifics. Note that two of the three files are in a win32 subdirectory, intended only for that platform. Shouldn't the "immediate compile error/problem" and "7 likely other compiler problems" manifest themselves in at least one trunk thread test failure over the past three months (since it was merged into the trunk)? Does this assuage your concerns?
participants (3)
-
Anthony Williams -
Jim Bell -
Rene Rivera