[optional] New RC_1_34_0 regression
I've just found what I believe to be a new regression on the boost RC_1_34_0 branch relative to the 1.33.1 release. The example below is a simplified extract of some real world where the behaviour of boost::none silently changes. #include <string> #include <iostream> #include <boost/optional.hpp> #include <boost/none.hpp> namespace sql { void literal( std::ostream& os, int i ) { os << i; } void literal( std::ostream& os, boost::optional<std::string> const& str ) { // escaping removed for clarity if (str) os << '"' << str.get() << '"'; else os << "NULL"; } } int main() { sql::literal( std::cout, boost::none ); } In 1.33.1, this prints out 'NULL' (which is what I would naively expect); on 1.34, it prints '0'. In both cases compiles cleanly in gcc 4.1.1. Sorry if this is a known regression, but having just seen the email saying RC_1_34_0 is down to zero regressions, I thought a quick-heads up was in order. Richard
AMDG Richard Smith <richard <at> ex-parrot.com> writes:
I've just found what I believe to be a new regression on the boost RC_1_34_0 branch relative to the 1.33.1 release. The example below is a simplified extract of some real world where the behaviour of boost::none silently changes.
<snip>
Doesn't internal linkage in headers have the same problem as unnamed namespaces? Is there some reason we can't use namespace boost { namespace detail { struct none_helper {}; }; typedef int (*none_t)(none_helper); inline int none(none_helper) {} } In Christ, Steven Watanabe
Steven Watanabe wrote:
Richard Smith <richard <at> ex-parrot.com> writes:
I've just found what I believe to be a new regression on the boost RC_1_34_0 branch relative to the 1.33.1 release. The example below is a simplified extract of some real world where the behaviour of boost::none silently changes.
Doesn't internal linkage in headers have the same problem as unnamed namespaces? Is there some reason we can't use
[...] This works for me: namespace boost { namespace detail { struct none_helper {}; } typedef int (*none_t)(detail::none_helper); inline int none(detail::none_helper) { return 0; } } (Which is your version with the obvious typos and warnings corrected.) That removes the regression I was describing. But I don't have ready access to one of the compilers (Borland or VC7.1 ?) that exhibits problems with precompiled headers. Richard Smith
Richard Smith wrote:
I've just found what I believe to be a new regression on the boost RC_1_34_0 branch relative to the 1.33.1 release. The example below is a simplified extract of some real world where the behaviour of boost::none silently changes.
[...] The attached patch includes a Steven Watanabe's proposed reimplementation of boost::none and a test case demonstrating the regression from 1.33.1. (1.33.1 passes this test case.) I know that it's late in the day for fixes on the 1.34 branch, but this is silent change in behaviour in a fairly straightforward usage of a commonly used boost library. I really do think it needs to be fixed before 1.34 can be released. Richard Smith
Richard Smith wrote:
Richard Smith wrote:
I've just found what I believe to be a new regression on the boost RC_1_34_0 branch relative to the 1.33.1 release. The example below is a simplified extract of some real world where the behaviour of boost::none silently changes.
[...]
The attached patch includes a Steven Watanabe's proposed reimplementation of boost::none and a test case demonstrating the regression from 1.33.1. (1.33.1 passes this test case.)
I know that it's late in the day for fixes on the 1.34 branch, but this is silent change in behaviour in a fairly straightforward usage of a commonly used boost library. I really do think it needs to be fixed before 1.34 can be released.
I disagree. There's no way we can release anything this century if we keep on checking in things are branch is frozen. The time window for commits is over at this point. If this patch breaks one of release compilers, we'll spend another week just to get back to zero regressions. The final decision is up to Thomas, of course, but I hope that no code changes will be allowed, no matter what. - Volodya
Vladimir Prus wrote:
Richard Smith wrote:
I know that it's late in the day for fixes on the 1.34 branch, but this is silent change in behaviour in a fairly straightforward usage of a commonly used boost library. I really do think it needs to be fixed before 1.34 can be released.
I disagree. There's no way we can release anything this century if we keep on checking in things are branch is frozen. The time window for commits is over at this point. If this patch breaks one of release compilers, we'll spend another week just to get back to zero regressions.
I understand the sentiment --- it has been a *long* time in getting to this point, and like everyone else I want to see 1.34 released as soon as possible. But I don't agree that this should prevent us from fixing this bug. My proposed patch is basically a reversion of those headers to their state prior to the Thu Mar 1 23:08:33 2007 UTC commit by Fernando Cacciola, but with Steven Watanabe's attempt to fix their known problem with precompiled headers. In their old state (i.e. before Mar 1st), <boost/none.hpp> caused compiler errors when used in precompiled headers in two popular compilers. And there was a workaround (don't use them in precompiled headers). In their current state, any standards compliant compiler will preferentially convert boost::none to int rather than boost::optional. (I believe all support compilers do this.) This is a silent change that will affect existing code -- I know it does, because I've found several affected uses in my own code. I would much rather see a release with this one header causing compiler errors when used in precompiled headers, than with a silent change in meaning to wellformed code. At least a user is forced to acknowledge a compiler error -- a silent change in meaning relies on the end users' unit tests noticing it. The fact that only the former error is caught by the testsuite doesn't, in my opinion, mean the latter is any less of a regression -- rather it's a deficiency in the currrent testsuite.
The final decision is up to Thomas, of course, but I hope that no code changes will be allowed, no matter what.
In that case, I think we disagree. Richard Smith
----Original Message---- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Richard Smith Sent: 04 April 2007 11:09 To: boost@lists.boost.org Subject: Re: [boost] [optional] New RC_1_34_0 regression
I understand the sentiment --- it has been a *long* time in getting to this point, and like everyone else I want to see 1.34 released as soon as possible.
On that I think EVERYBODY agrees!
But I don't agree that this should prevent us from fixing this bug.
I have to say I think I am inclining to your view. This is a) a genuine regresssion from 1.33.1. User code that used to work no longer does so. b) The worst sort of "does not work"; there is a silent change in behaviour.
The final decision is up to Thomas, of course, Heh. Another point on which I think we all agree.
It's at times like this I'm glad I'm not the release manager! -- Martin Bonner Project Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com
"Vladimir Prus" <ghost@cs.msu.su> wrote in message news:E1HZ27f-0007ib-DI@zigzag.lvk.cs.msu.su...
I disagree. There's no way we can release anything this century if we keep on checking in things are branch is frozen. The time window for commits is over at this point. If this patch breaks one of release compilers, we'll spend another week just to get back to zero regressions.
The final decision is up to Thomas, of course, but I hope that no code changes will be allowed, no matter what.
- Volodya
I just want to add my 1.5 cents to this. I want to see this build get released as much as anyone. But, let us not confuse the regression test's output of no regressions with the fact that there is in fact a regression. The only reason that the regression test is currently showing zero errors is, in light of this find, because it is clearly incomplete, since a regression not being tested for was in fact found. So, for my 1.5 cents of input, I would suggest that the patch be made, as long as no other errors in our current regression test are caused by it. If it is, in fact, as transparent to the rest of the regression test as claimed, I see no reason why it shouldn't go in. If it breaks anything, and I mean anything for any compiler, an argument can be made for rolling it back and releasing without it. Thanks, Michael Goldshteyn
Michael Goldshteyn wrote:
"Vladimir Prus" <ghost@cs.msu.su> wrote in message news:E1HZ27f-0007ib-DI@zigzag.lvk.cs.msu.su...
I disagree. There's no way we can release anything this century if we keep on checking in things are branch is frozen. The time window for commits is over at this point. If this patch breaks one of release compilers, we'll spend another week just to get back to zero regressions.
The final decision is up to Thomas, of course, but I hope that no code changes will be allowed, no matter what.
- Volodya
I just want to add my 1.5 cents to this. I want to see this build get released as much as anyone. But, let us not confuse the regression test's output of no regressions with the fact that there is in fact a regression.
Indeed! Because there's no "output of no regressions". The value of zero reported currently is the value of *unexpected* failures. If a failure is marked as expected, it's not reported, even if that failures was not present in Boost 1.33.1. I'm sure that there are regressions that are marked expected at the moment. Previously, we decided on this list that on a certain date, all remaining failures will be marked as expected. Now, you're pointing out that we have a regression, for which we don't even have a test. In light of that decision, we probably can add a test for that problem, and immediately mark all failures of said test as expected. But that would not be very helpful. I don't think we should give this problem any bonus points just because it's not discovered by the tests yet. - Volodya
Vladimir Prus wrote:
Previously, we decided on this list that on a certain date, all remaining failures will be marked as expected.
Freeze was scheduled to be at 11:00 UTC on Mar 2nd; the regression was only introduced at 23:08 UTC on Mar 1st. And the file in question (<boost/none.hpp>) has been modified twice since then to fix other regressions introduced by that commit. Don't get me wrong -- I'm not against that commit having gone in at the last minute -- other parts of the commit fixed several important issues, but it seems unreasonable to expect any problems with it to have been fixed in the twelve hours between commit and freeze. And indeed the file in question has been edited in the last week to fix other regressions.
Now, you're pointing out that we have a regression, for which we don't even have a test.
Yes you do. The patch I attached to my email this morning added a test to the test suite.
In light of that decision, we probably can add a test for that problem, and immediately mark all failures of said test as expected. But that would not be very helpful. I don't think we should give this problem any bonus points just because it's not discovered by the tests yet.
Indeed, and I'm not suggesting it should. But if the test suites were indiciating a failure that would silently change legitimate user code across all platforms, I would hope that too would be fixed rather than marked 'expected'. Richard
ping Thomas ----Original Message---- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Richard Smith Sent: 04 April 2007 16:48 To: Vladimir Prus Subject: Re: [boost] [optional] New RC_1_34_0 regression
Vladimir Prus wrote:
Previously, we decided on this list that on a certain date, all remaining failures will be marked as expected.
Freeze was scheduled to be at 11:00 UTC on Mar 2nd; the regression was only introduced at 23:08 UTC on Mar 1st. And the file in question (<boost/none.hpp>) has been modified twice since then to fix other regressions introduced by that commit.
Don't get me wrong -- I'm not against that commit having gone in at the last minute -- other parts of the commit fixed several important issues, but it seems unreasonable to expect any problems with it to have been fixed in the twelve hours between commit and freeze. And indeed the file in question has been edited in the last week to fix other regressions.
Now, you're pointing out that we have a regression, for which we don't even have a test.
Yes you do. The patch I attached to my email this morning added a test to the test suite.
In light of that decision, we probably can add a test for that problem, and immediately mark all failures of said test as expected. But that would not be very helpful. I don't think we should give this problem any bonus points just because it's not discovered by the tests yet.
Indeed, and I'm not suggesting it should. But if the test suites were indiciating a failure that would silently change legitimate user code across all platforms, I would hope that too would be fixed rather than marked 'expected'.
Richard _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Martin Bonner Project Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com
Might I add a possible third alternative? Why not make the patches publicly available on boost's web site? Something like Microsoft's hot-fixes. That way the release goes forward and if a user if effected by this they can patch their own copy. Having such a facility would also help in the future for addressing major problems between release, if a user has a problem they could scan the patches and say "oh here's a patch that fixes the problem I'm having" apply it and move on. Food for thought perhaps? Matt
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Martin Bonner Sent: Wednesday, April 04, 2007 9:57 AM To: boost@lists.boost.org Subject: Re: [boost] [1.34.0] (was [optional] New RC_1_34_0 regression)
ping Thomas
----Original Message---- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Richard Smith Sent: 04 April 2007 16:48 To: Vladimir Prus Subject: Re: [boost] [optional] New RC_1_34_0 regression
Vladimir Prus wrote:
Previously, we decided on this list that on a certain date, all remaining failures will be marked as expected.
Freeze was scheduled to be at 11:00 UTC on Mar 2nd; the regression was only introduced at 23:08 UTC on Mar 1st. And the file in question (<boost/none.hpp>) has been modified twice since then to fix other regressions introduced by that commit.
Don't get me wrong -- I'm not against that commit having gone in at the last minute -- other parts of the commit fixed several important issues, but it seems unreasonable to expect any problems with it to have been fixed in the twelve hours between commit and freeze. And indeed the file in question has been edited in the last week to fix other regressions.
Now, you're pointing out that we have a regression, for which we don't even have a test.
Yes you do. The patch I attached to my email this morning added a test to the test suite.
In light of that decision, we probably can add a test for that problem, and immediately mark all failures of said test as expected. But that would not be very helpful. I don't think we should give this problem any bonus points just because it's not discovered by the tests yet.
Indeed, and I'm not suggesting it should. But if the test suites were indiciating a failure that would silently change legitimate user code across all platforms, I would hope that too would be fixed rather than marked 'expected'.
Richard _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
--
Martin Bonner Project Leader
PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Scanned by McAfee GroupShield {X3BTB534}
Vladimir Prus 写道:
Richard Smith wrote:
Richard Smith wrote:
I've just found what I believe to be a new regression on the boost RC_1_34_0 branch relative to the 1.33.1 release. The example below is a simplified extract of some real world where the behaviour of boost::none silently changes. [...]
The attached patch includes a Steven Watanabe's proposed reimplementation of boost::none and a test case demonstrating the regression from 1.33.1. (1.33.1 passes this test case.)
I know that it's late in the day for fixes on the 1.34 branch, but this is silent change in behaviour in a fairly straightforward usage of a commonly used boost library. I really do think it needs to be fixed before 1.34 can be released.
I disagree. There's no way we can release anything this century if we keep on checking in things are branch is frozen. The time window for commits is over at this point. If this patch breaks one of release compilers, we'll spend another week just to get back to zero regressions. We have spent another week, and the patch was just submitted.
The final decision is up to Thomas, of course, but I hope that no code changes will be allowed, no matter what.
- Volodya
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hi Richard, I' glad you found this regression now, _before_ the release. FWIW I agree that that 1.34 should ship with an non-breaking implementation of none. As you know the intention was to fix a long standing problem with precompiled headers, but by no means that fix can existing code. I will contact the Release Manager (Thomas) asking for permission to revert the fix that caused the regresion. I will patch it by physically reverting the problematic fix, from CVS history, rather than patching the code one more time, to make sure there are no further mistakes. Best Fernando Cacciola
Hi, I convinced myself that this needs to be fixed before the release. While I am sympathetic to Vladimirs argument this seems serious enough to me to call it a showstopper. Fernando Cacciola wrote:
Hi Richard,
I' glad you found this regression now, _before_ the release.
FWIW I agree that that 1.34 should ship with an non-breaking implementation of none. As you know the intention was to fix a long standing problem with precompiled headers, but by no means that fix can existing code.
I will contact the Release Manager (Thomas) asking for permission to revert the fix that caused the regresion.
I will patch it by physically reverting the problematic fix, from CVS history, rather than patching the code one more time, to make sure there are no further mistakes.
Please go ahead. It's your call whether to simply revert or accept Richard's patch. The way I look at it is that the patch has higher risk but a nice testcase, that makes it even to me. Thomas -- Thomas Witt witt@acm.org
Thomas Witt wrote:
I will contact the Release Manager (Thomas) asking for permission to revert the fix that caused the regresion.
I will patch it by physically reverting the problematic fix, from CVS history, rather than patching the code one more time, to make sure there are no further mistakes.
Please go ahead. It's your call whether to simply revert or accept Richard's patch. The way I look at it is that the patch has higher risk but a nice testcase, that makes it even to me.
For avoidance of doubt -- Richard, do you have commit access. If no, we have two question -- which solution to apply (Richard, can you indicate that) and who gets to check it in. - Volodya
Vladimir Prus wrote:
For avoidance of doubt -- Richard, do you have commit access.
No.
If no, we have two question -- which solution to apply (Richard, can you indicate that) and who gets to check it in.
I took Thomas' email to mean the decision was Fernando's, and it would be him (Fernando) who would commit. I don't have strong opinions on whether we revert or apply my patch. Reverting will definitely break precompiled headers involving <boost/none.hpp> on VC7.1 and Borland; my patch contains Steven Watanabe's attempt to solve this (which I haven't been able to verify as I don't have access to either affected platform, though it compiles and passes its tests on other systems), and includes a test case for the original regression, but it could introduce a regression on some other platforms. If Fernando thinks it's safest to simply revert, that's absolutely fine by me. Cheers Richard
I took Thomas' email to mean the decision was Fernando's, and it would be him (Fernando) who would commit.
Indeed. I will commit sometime within the next hour.
I don't have strong opinions on whether we revert or apply my patch. Reverting will definitely break precompiled headers involving <boost/none.hpp> on VC7.1 and Borland; my patch contains Steven Watanabe's attempt to solve this (which I haven't been able to verify as I don't have access to either affected platform, though it compiles and passes its tests on other systems), and includes a test case for the original regression, but it could introduce a regression on some other platforms.
If Fernando thinks it's safest to simply revert, that's absolutely fine by me.
I don't have access to those compiler either, so there is some uncertainty in the patch, even if not w.r.t to regressions. So given that I can't test if it really works (in terms of making PCHs happy), I rather just revert. But of course I will apply it, along with the test case, in HEAD, for 1.35. Best Fernando
"Fernando Cacciola" <fernando_cacciola@hotmail.com> wrote in message news:ev30kg$7ql$1@sea.gmane.org...
I don't have access to those compiler either, so there is some uncertainty in the patch, even if not w.r.t to regressions. So given that I can't test if it really works (in terms of making PCHs happy), I rather just revert.
But of course I will apply it, along with the test case, in HEAD, for 1.35.
Best
Fernando
This is unclear, what are you reverting to? Michael Goldshteyn
Hi Thomas,
Please go ahead. It's your call whether to simply revert or accept Richard's patch. The way I look at it is that the patch has higher risk but a nice testcase, that makes it even to me.
Well, it turns out that reverting isn't as straightforward as I figured. The problem is that the previous version used two separate headers "none_t.hpp" and "none.hpp", with only the first being included by "optional.hpp", forcing users to include "none.hpp" manually. That separation was simply intended to allow Borland users to work around the PCH problem. But only so much. A nice feature of the "new" implementation (the one that caused all this trouble) is that you no longer need to include "none.hpp" manually anymore if you use optional<>. But I really hated the idea of going back to 2 headers, so I spent quite some time trying to make an informed decision. First, I searched the list archives for threads about 'none'. Interestingly enough, I found this: http://thread.gmane.org/gmane.comp.lib.boost.devel/146009/focus=146126 There you can see the proposal for the alternative implementation, which eventually I put in place and casused all this, but, you can also see that at _that_ time, I was much much more lucid that in the recent days and figured that it would cause the issue Richard just found. Furthermore, in that thread there is a different implementation proposed by Anthony Williams which, incidentally, is the same Steven just posted here (that code is slightly more elaborated, but the principle is the same). You can also read that I agreed at the time with Anthony's implementation (...but then my clone forgot about it and picked the wrong one to commit ;) Then I tried out Anthony's version (attached here) with the Optional testsuite but also with a new specific test (derived from Richard's post here, see attached). The tests pass with toolsets msvc-8.00 and msvc-7.1 in WinXP. I can test it with gcc (4.4.1) in a Debian Linux Box, but not today (8pm already here). And probably not until next Monday. I installed the command line version of borland 5.5.1, but when I run "bjam --toolset=borland", bjam crashes, so I couldn't test there. Can those participating in this thread take a look at the new implementation, test if you can, and report back so we can decide if it is safe to commit? TIA ======================================================================= // Copyright (C) 2003, Fernando Luis Cacciola Carballal. // Copyright (C) 2007, Anthony Williams // // Distributed under the Boost Software License, Version 1.0. // (See accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) // // See http://www.boost.org/lib/optional/ for documentation. // // You are welcome to contact the author at: // fernando.cacciola@gmail.com // #ifndef BOOST_NONE_17SEP2003_HPP #define BOOST_NONE_17SEP2003_HPP namespace boost { namespace detail { class none_helper; } inline void none(detail::none_helper); namespace detail { class none_helper { private: none_helper( none_helper const& ) {} friend void boost::none(none_helper); }; } typedef void (*none_t)(detail::none_helper); inline void none(detail::none_helper) {} } #endif none_test.cpp: ======================================= #include <boost/optional/optional.hpp> // Test that none.hpp is included with <optional.hpp> // Left undefined to cause a linker error if this overload is incorrectly selected. void verify_no_implicit_conversion_to_int ( int i ) ; void verify_no_implicit_conversion_to_int ( boost::optional<int> const& ) {} int main() { verify_no_implicit_conversion_to_int( boost::none ); } =======================================
Fernando Cacciola wrote:
Fernando Cacciola wrote:
I can test it with gcc (4.4.1) in a Debian Linux Box, but not today (8pm already here). And probably not until next Monday.
OK. I just did it, and it also works with gcc 4.4.1
I'm certainly missing something. Clearly, the change you committed right before freeze changed a finite amount of files. What's wrong with reverting those files into the state they had immediately before your commit? While "I really hated the idea of going back to 2 headers" might be a good technical argument at a different time, do we really want to spend time trying yet another alternative solution at this point? - Volodya
Fernando Cacciola wrote:
I can test it with gcc (4.4.1) in a Debian Linux Box, but not today (8pm already here). And probably not until next Monday.
OK. I just did it, and it also works with gcc 4.4.1
I'm certainly missing something. Clearly, the change you committed right before freeze changed a finite amount of files. What's wrong with reverting those files into the state they had immediately before your commit?
While "I really hated the idea of going back to 2 headers" might be a good technical argument at a different time, do we really want to spend time trying yet another alternative solution at this point?
If it were just a matter of reverting N files to a previous version I would just do it. But it isn't. I need to revert 2 files, BUT ALSO 1.Manually edit-back "optional.hpp", which included "none_t.hpp" but now includes "none.hpp" 2.Manually edit-back those test files which included "none.hpp" explicitly and now they doesn't. 3.Manually re-edit the documentation to state that "none.hpp" must be included separately. Granted, I can avoid 1,2 and 3 there by going back to the 2 headers but keeping "optional.hpp" *updated*, that is, including "none.hpp". But that renders the point of having two separate headers totally useless. I totally understand that doing a last minute fix which delays the release even longer is annoying, but IMHO is even more annoying to release something knowing it has a problem just because we don't want to spend a few additional days clearing up anything that may leak after the fix. I'm afraid that's just the way our industry works, but I don't but like it. Also, unlike the problematic in-a-rush fix that caused the problem before, I've given this fix a lot of attention, and it comes with a test, so I could tested it locally on 3 different platforms and I will test it on a fourth today. And finally, Thomas is off until Wednesday anyway, so what's the point in rushing just now when we can do whatever it takes to make sure this fix will really work? Best Fernando
Fernando Cacciola wrote:
Fernando Cacciola wrote:
I can test it with gcc (4.4.1) in a Debian Linux Box, but not today (8pm already here). And probably not until next Monday.
OK. I just did it, and it also works with gcc 4.4.1
I'm certainly missing something. Clearly, the change you committed right before freeze changed a finite amount of files. What's wrong with reverting those files into the state they had immediately before your commit?
While "I really hated the idea of going back to 2 headers" might be a good technical argument at a different time, do we really want to spend time trying yet another alternative solution at this point?
If it were just a matter of reverting N files to a previous version I would just do it. But it isn't.
I need to revert 2 files, BUT ALSO
1.Manually edit-back "optional.hpp", which included "none_t.hpp" but now includes "none.hpp"
If it included none_t.hpp and now includes none.hpp, it means there was a commit which changed that file. How about reverting that commit?
2.Manually edit-back those test files which included "none.hpp" explicitly and now they doesn't.
Why? Weren't those file changed as part of some commit?
3.Manually re-edit the documentation to state that "none.hpp" must be included separately.
Again, I'm lost. I presume there was commit which made documentation no longer say that none.hpp must be included separately.
Granted, I can avoid 1,2 and 3 there by going back to the 2 headers but keeping "optional.hpp" *updated*, that is, including "none.hpp". But that renders the point of having two separate headers totally useless.
I totally understand that doing a last minute fix which delays the release even longer is annoying, but IMHO is even more annoying to release something knowing it has a problem just because we don't want to spend a few additional days clearing up anything that may leak after the fix. I'm afraid that's just the way our industry works, but I don't but like it.
Also, unlike the problematic in-a-rush fix that caused the problem before, I've given this fix a lot of attention, and it comes with a test, so I could tested it locally on 3 different platforms and I will test it on a fourth today.
And finally, Thomas is off until Wednesday anyway, so what's the point in rushing just now when we can do whatever it takes to make sure this fix will really work?
Well, technically Thomas said it's OK to either revert, or apply Richard's patch. You seem to propose checking it some different patch. Assuming you post it now, and Thomas OKs it on Wednesday, and there are no issues, we've lost 2 days compared to reverting today. And delays tend to add up. I don't have no authority to tell you what to do; if you feel that whatever patch you have now is OK, you should post it and wait for Thomas to comment. - Volodya
Vladimir Prus wrote:
Fernando Cacciola wrote:
Fernando Cacciola wrote:
I can test it with gcc (4.4.1) in a Debian Linux Box, but not today (8pm already here). And probably not until next Monday.
OK. I just did it, and it also works with gcc 4.4.1
I'm certainly missing something. Clearly, the change you committed right before freeze changed a finite amount of files. What's wrong with reverting those files into the state they had immediately before your commit?
While "I really hated the idea of going back to 2 headers" might be a good technical argument at a different time, do we really want to spend time trying yet another alternative solution at this point?
If it were just a matter of reverting N files to a previous version I would just do it. But it isn't.
I need to revert 2 files, BUT ALSO
1.Manually edit-back "optional.hpp", which included "none_t.hpp" but now includes "none.hpp"
If it included none_t.hpp and now includes none.hpp, it means there was a commit which changed that file. How about reverting that commit?
That would revert all the other changes to "optional.hpp" (which is the bulk of the update and I definitely won't roll-back).
2.Manually edit-back those test files which included "none.hpp" explicitly and now they doesn't.
Why? Weren't those file changed as part of some commit?
Yes, a commit that involved other changes that I don't want to roll-back.
3.Manually re-edit the documentation to state that "none.hpp" must be included separately.
Again, I'm lost. I presume there was commit which made documentation no longer say that none.hpp must be included separately.
Sort of. The doc commit introduced all the updates to optional, including "none" documentation which was entirely non-existing before. As you can see now, the problem is that the problematic commit did not involve just "none" but a lot of other changes.
Granted, I can avoid 1,2 and 3 there by going back to the 2 headers but keeping "optional.hpp" updated, that is, including "none.hpp". But that renders the point of having two separate headers totally useless.
I totally understand that doing a last minute fix which delays the release even longer is annoying, but IMHO is even more annoying to release something knowing it has a problem just because we don't want to spend a few additional days clearing up anything that may leak after the fix. I'm afraid that's just the way our industry works, but I don't but like it.
Also, unlike the problematic in-a-rush fix that caused the problem before, I've given this fix a lot of attention, and it comes with a test, so I could tested it locally on 3 different platforms and I will test it on a fourth today.
And finally, Thomas is off until Wednesday anyway, so what's the point in rushing just now when we can do whatever it takes to make sure this fix will really work?
Well, technically Thomas said it's OK to either revert, or apply Richard's patch. You seem to propose checking it some different patch.
If you look at it you'll see is only marginally different.
Assuming you post it now, and Thomas OKs it on Wednesday, and there are no issues, we've lost 2 days compared to reverting today. And delays tend to add up.
Why 'lost'? To me we gained two days to make sure the patch works.
I don't have no authority to tell you what to do; if you feel that whatever patch you have now is OK, you should post it and wait for Thomas to comment.
I already posted it 4 days ago, and asked everyone in this thread to test it if they can. The bottom line is that I'm pretty confident this patch will work. It's the same patch Richard proposed (go look at it and you'll see that, even if not token-by-token, it is the same), I used a specific test-case, and the test case passed the tests in 3 platforms. Best Fernando
Fernando Cacciola wrote:
2.Manually edit-back those test files which included "none.hpp" explicitly and now they doesn't.
Why? Weren't those file changed as part of some commit?
Yes, a commit that involved other changes that I don't want to roll-back. ... As you can see now, the problem is that the problematic commit did not involve just "none" but a lot of other changes.
Ok, I can rumble about atomic commits for a while, but it's pointless now.
I don't have no authority to tell you what to do; if you feel that whatever patch you have now is OK, you should post it and wait for Thomas to comment.
I already posted it 4 days ago, and asked everyone in this thread to test it if they can.
Ok, if you've made no change to that nope.hpp you've posted, then we can only wait for Thomas to comment. - Volodya
Vladimir Prus wrote:
Fernando Cacciola wrote:
[...]
I already posted it 4 days ago, and asked everyone in this thread to test it if they can.
Ok, if you've made no change to that nope.hpp you've posted, then we can only wait for Thomas to comment.
Sorry to drop in, but... Thomas is away until Wednesday. If I understood the rest of the discussion correctly, it has already been decided that this should be fixed in some way or another. So you might as well commit now, and roll back on Wednesday if Thomas decides otherwise. But at least the testing time is not lost until then. Markus
Markus Schöpflin wrote:
Vladimir Prus wrote:
Fernando Cacciola wrote:
[...]
I already posted it 4 days ago, and asked everyone in this thread to test it if they can.
Ok, if you've made no change to that nope.hpp you've posted, then we can only wait for Thomas to comment.
Sorry to drop in, but...
Thomas is away until Wednesday. If I understood the rest of the discussion correctly, it has already been decided that this should be fixed in some way or another.
So you might as well commit now, and roll back on Wednesday if Thomas decides otherwise. But at least the testing time is not lost until then.
Hmm, this is a good point. If I commit now we'll have one day of full regressions to look at. Best Fernando
Fernando Cacciola wrote:
Markus Schöpflin wrote:
So you might as well commit now, and roll back on Wednesday if Thomas decides otherwise. But at least the testing time is not lost until then.
Hmm, this is a good point. If I commit now we'll have one day of full regressions to look at.
FWIW, this will trigger a rather long regression test run; the results will likely not be uploaded before Thursday. Given the current frequency of result table regenerations, the results may well not be visible before Friday. Regards, m
Martin Wille wrote:
Fernando Cacciola wrote:
Markus Schöpflin wrote:
So you might as well commit now, and roll back on Wednesday if Thomas decides otherwise. But at least the testing time is not lost until then.
Hmm, this is a good point. If I commit now we'll have one day of full regressions to look at.
FWIW, this will trigger a rather long regression test run
Another good point.
the results will likely not be uploaded before Thursday. Given the current frequency of result table regenerations, the results may well not be visible before Friday.
OK. Then there's no point committing now. Best Fernando
Fernando Cacciola wrote:
--===============0404217746==
Martin Wille wrote:
the results will likely not be uploaded before Thursday. Given the current frequency of result table regenerations, the results may well not be visible before Friday.
OK. Then there's no point committing now.
That delay in inherent in the system though, deferring the commit simply delays that next set of results even more. If we really want this patch (and I think we do) then I would suggest the earlier we get it tested the better. This appears to be the last known issue, so the sooner we get it resolved the closer we are to a high quality Release Candidate. -- AlisdairM (still glad it is not his decision to make!)
Fernando, Please commit your changes as soon as possible. Thanks Thomas Fernando Cacciola wrote:
Martin Wille wrote:
Fernando Cacciola wrote:
Markus Schöpflin wrote:
So you might as well commit now, and roll back on Wednesday if Thomas decides otherwise. But at least the testing time is not lost until then.
Hmm, this is a good point. If I commit now we'll have one day of full regressions to look at. FWIW, this will trigger a rather long regression test run
Another good point.
the results will likely not be uploaded before Thursday. Given the current frequency of result table regenerations, the results may well not be visible before Friday.
OK. Then there's no point committing now.
Best
Fernando
------------------------------------------------------------------------
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Thomas Witt witt@acm.org
Fernando Cacciola wrote:
Thomas Witt wrote:
Fernando,
Please commit your changes as soon as possible.
Done.
After an update I got lots of warnings of the form: ./boost/none.hpp:43:7: warning: no newline at end of file so I committed the obvious fix (after 'review' on IRC). FYI. Regards, Stefan -- ...ich hab' noch einen Koffer in Berlin...
participants (13)
-
AlisdairM -
Atry -
Fernando Cacciola -
Markus Schöpflin -
Martin Bonner -
Martin Wille -
Matt Doyle -
Michael Goldshteyn -
Richard Smith -
Stefan Seefeld -
Steven Watanabe -
Thomas Witt -
Vladimir Prus