Review request: Require a SFINAE capable compiler
Hello, Several of the ancient compilers define BOOST_NO_SFINAE. The attached patch removes support for those versions of the compilers which are not capable of SFINAE. This bumps the compiler requirement to remove support for Borland 5.x, Metrowerks prior to 9.0, and SunPro prior to 5.7. Compilers based on EDG 238 are also affected, though it is unknown what those are. Ok to commit? Thanks, Steve.
On 10/12/2013 10:06 AM, Stephen Kelly wrote:
// versions check: // we don't support Metrowerks prior to version 5.3: -#if __MWERKS__ < 0x2301 +#if __MWERKS__ < 0x3207 # error "Compiler not supported or configured - please reconfigure" #endif //
I've fixed this locally to be // versions check: -// we don't support Metrowerks prior to version 5.3: -#if __MWERKS__ < 0x2301 +// we don't support Metrowerks prior to version 9.0: +#if __MWERKS__ < 0x3200 # error "Compiler not supported or configured - please reconfigure" #endif Thanks, Steve.
On 10/12/2013 10:06 AM, Stephen Kelly wrote:
Hello,
Several of the ancient compilers define BOOST_NO_SFINAE.
The attached patch removes support for those versions of the compilers which are not capable of SFINAE.
This bumps the compiler requirement to remove support for Borland 5.x, Metrowerks prior to 9.0, and SunPro prior to 5.7. Compilers based on EDG 238 are also affected, though it is unknown what those are.
#error "Compiler not supported or configured - please reconfigure"
Should this not be replaced with something that does not give the affected user false hope that reconfiguration rather than compiler upgrade will help him to overcome this? -- VZ
On 10/13/2013 08:37 AM, Václav Zeman wrote:
#error "Compiler not supported or configured - please reconfigure" Should this not be replaced with something that does not give the affected user false hope that reconfiguration rather than compiler upgrade will help him to overcome this?
Yes, I'll try to clarify that. Thanks, Steve.
Le 12/10/13 10:06, Stephen Kelly a écrit :
Hello,
Several of the ancient compilers define BOOST_NO_SFINAE.
The attached patch removes support for those versions of the compilers which are not capable of SFINAE.
This bumps the compiler requirement to remove support for Borland 5.x, Metrowerks prior to 9.0, and SunPro prior to 5.7. Compilers based on EDG 238 are also affected, though it is unknown what those are.
Ok to commit?
Please, wait for at least John give you permission. BTW, Could you add the patch to the documentation concerning BOOST_NO_SFINAE has been removed. How many libraries are concerned by this change? Could you give the list so that the authors/maintainers of these libraries could take a look at your patch? Best, Vicente
Le 13/10/13 13:00, Vicente J. Botet Escriba a écrit :
Le 12/10/13 10:06, Stephen Kelly a écrit :
Hello,
Several of the ancient compilers define BOOST_NO_SFINAE.
The attached patch removes support for those versions of the compilers which are not capable of SFINAE.
This bumps the compiler requirement to remove support for Borland 5.x, Metrowerks prior to 9.0, and SunPro prior to 5.7. Compilers based on EDG 238 are also affected, though it is unknown what those are.
Ok to commit?
Please, wait for at least John give you permission.
BTW, Could you add the patch to the documentation concerning BOOST_NO_SFINAE has been removed.
How many libraries are concerned by this change? Could you give the list so that the authors/maintainers of these libraries could take a look at your patch?
Just a minor typo # if(__MWERKS__ <= 0x3207) || !defined(BOOST_STRICT_CONFIG) // 9.6 @@ -129,7 +109,7 @@ // // versions check: // we don't support Metrowerks prior to version 5.3: -#if __MWERKS__ < 0x2301 +#if __MWERKS__ < 0x3207 # error "Compiler not supported or configured - please reconfigure" #endif This comment // we don't support Metrowerks prior to version 5.3: should be // we don't support Metrowerks prior to version 9.6, isn't it?: Why this line -#define BOOST_NO_SFINAE has been removed in diff --git a/boost/config/compiler/digitalmars.hpp b/boost/config/compiler/digitalmars.hpp index 7de6adb..20c2dcc 100644 --- a/boost/config/compiler/digitalmars.hpp +++ b/boost/config/compiler/digitalmars.hpp @@ -15,7 +15,6 @@ #define BOOST_NO_MEMBER_TEMPLATE_FRIENDS #define BOOST_NO_OPERATORS_IN_NAMESPACE #define BOOST_NO_UNREACHABLE_RETURN_DETECTION -#define BOOST_NO_SFINAE #define BOOST_NO_USING_TEMPLATE #define BOOST_FUNCTION_SCOPE_USING_DECLARATION_BREAKS_ADL #endif Best, Vicente
The attached patch removes support for those versions of the compilers which are not capable of SFINAE.
This bumps the compiler requirement to remove support for Borland 5.x, Metrowerks prior to 9.0, and SunPro prior to 5.7. Compilers based on EDG 238 are also affected, though it is unknown what those are.
Ok to commit?
Please, wait for at least John give you permission.
It's OK by me, though as others have already said, best to wait for the 1.55 release to go out first... just in case we need to test patches/bug reports in Trunk. I suspect we've also reached the point where deprecating many more workarounds will likely cause a lot of flak once users figure out what we're doing. I guess in an ideal world, we would advertise what's being removed on the home page for 6 months or so before actually doing it. But I recognize that we may just need to move forward while we have a volunteer willing to do the work. Cheers, John.
On 10/13/2013 02:17 PM, John Maddock wrote:
The attached patch removes support for those versions of the compilers which are not capable of SFINAE.
This bumps the compiler requirement to remove support for Borland 5.x, Metrowerks prior to 9.0, and SunPro prior to 5.7. Compilers based on EDG 238 are also affected, though it is unknown what those are.
Ok to commit?
Please, wait for at least John give you permission.
It's OK by me, though as others have already said, best to wait for the 1.55 release to go out first... just in case we need to test patches/bug reports in Trunk.
This is what I don't understand. I don't see how my work makes this any harder. At *worst*, a bug fix will have a part in the positive side of an ifdef for some feature (like BOOST_NO_SFINAE), and a part in the negative side of it. I don't think that's even a likely scenario. Also, that's assuming the one who writes the patch patches both sides of the ifdef. That is still trivial to cherry-plck into the trunk branch. Or do you think it is not trivial? In that case, what makes it non-trivial? I want to understand the problem that exists and is stalling me. Thanks, Steve.
On Sunday 13 October 2013 20:26:56 Stephen Kelly wrote:
On 10/13/2013 02:17 PM, John Maddock wrote:
It's OK by me, though as others have already said, best to wait for the 1.55 release to go out first... just in case we need to test patches/bug reports in Trunk.
This is what I don't understand.
I don't see how my work makes this any harder. At *worst*, a bug fix will have a part in the positive side of an ifdef for some feature (like BOOST_NO_SFINAE), and a part in the negative side of it. I don't think that's even a likely scenario. Also, that's assuming the one who writes the patch patches both sides of the ifdef.
That is still trivial to cherry-plck into the trunk branch. Or do you think it is not trivial? In that case, what makes it non-trivial?
I want to understand the problem that exists and is stalling me.
While the release is in process of preparation, like now, it is possible that a bug is reported and fixed in trunk. The developer then waits to see if the tests are ok in order to merge the fix to release. Since the time is pressing and tests turnaround is typically around a week or so, even a brief breakage in trunk is critical. Such breakage can prevent a valid bug fix from making it into release. Further I'll just speak for myself. I prefer trunk and release branch to be always as close as possible. I don't do cherry picking when merging to release, I always synchronize release with trunk (after tests pass) by merging the whole library. This way I make sure I don't have any forgotten unmerged commits in trunk. This also makes sure that I mostly have a single version of the library everywhere, so I don't have to guess when someone reports a problem with the library. Cherry picking is also more complicated by itself since I have to figure out which commits I want to merge and which I don't (and why). So, I'm in favor of postponing any massive changes to Boost.Log or Boost.Atomic in trunk until after 1.55. That said, I'm not against the general trend of raising compiler requirements or someone patching the libraries I (co-)maintain. Perhaps, it would be a good idea to create tickets for the affected libraries with localized patches instead of posting here cumulative patches to the whole Boost. This way the library maintainers will be properly notified of the planned changes, the changes won't be lost and will be applied when it is convenient.
On 10/13/2013 09:31 PM, Andrey Semashev wrote:
On Sunday 13 October 2013 20:26:56 Stephen Kelly wrote:
On 10/13/2013 02:17 PM, John Maddock wrote:
It's OK by me, though as others have already said, best to wait for the 1.55 release to go out first... just in case we need to test patches/bug reports in Trunk. This is what I don't understand.
I don't see how my work makes this any harder. At *worst*, a bug fix will have a part in the positive side of an ifdef for some feature (like BOOST_NO_SFINAE), and a part in the negative side of it. I don't think that's even a likely scenario. Also, that's assuming the one who writes the patch patches both sides of the ifdef.
That is still trivial to cherry-plck into the trunk branch. Or do you think it is not trivial? In that case, what makes it non-trivial?
I want to understand the problem that exists and is stalling me. While the release is in process of preparation, like now, it is possible that a bug is reported and fixed in trunk. The developer then waits to see if the tests are ok in order to merge the fix to release. Since the time is pressing and tests turnaround is typically around a week or so, even a brief breakage in trunk is critical. Such breakage can prevent a valid bug fix from making it into release.
Further I'll just speak for myself. I prefer trunk and release branch to be always as close as possible. I don't do cherry picking when merging to release, I always synchronize release with trunk (after tests pass) by merging the whole library. This way I make sure I don't have any forgotten unmerged commits in trunk.
I won't comment on how good an idea this is, in my view... :)
This also makes sure that I mostly have a single version of the library everywhere, so I don't have to guess when someone reports a problem with the library.
If the reporter doesn't say, then you have to ask them. They might respond version 1.54, 1.53, 1.52 etc... or they might respond 'release branch' or 'trunk branch'. Either you guess or you ask.
Cherry picking is also more complicated by itself since I have to figure out which commits I want to merge and which I don't (and why).
As the maintainer, which ones you cherry-pick are simply your choice. I don't see how that is complicated either you think it should be in branch xyz, or you don't.
So, I'm in favor of postponing any massive changes to Boost.Log or Boost.Atomic in trunk until after 1.55.
Yes. I'm not going to make more changes, except on request until after 1.55 is out. Daniel said until 'some time after' 1.55 is out. I have no idea how much time he has in mind...
That said, I'm not against the general trend of raising compiler requirements or someone patching the libraries I (co-)maintain.
Great, thanks.
Perhaps, it would be a good idea to create tickets for the affected libraries with localized patches instead of posting here cumulative patches to the whole Boost. This way the library maintainers will be properly notified of the planned changes, the changes won't be lost and will be applied when it is convenient.
That's not practical. I'm changing many libraries, and I'm making micro-commits for easy review. I'm not going to file a ticket for each one. If you want to know how your library will be affected by my changes, then simply grep for the macros under discussion for removal (BOOST_NO_SFINAE, at least, currently). Thanks, Steve.
On Sunday 13 October 2013 22:03:30 Stephen Kelly wrote:
On 10/13/2013 09:31 PM, Andrey Semashev wrote:
Cherry picking is also more complicated by itself since I have to figure out which commits I want to merge and which I don't (and why).
As the maintainer, which ones you cherry-pick are simply your choice. I don't see how that is complicated either you think it should be in branch xyz, or you don't.
This implies I have to examine each of the unmerged commits to make that choice. And I have to do that after some time has passed since the commit was made. I find my synchronizing approach much simpler and less time consuming.
Perhaps, it would be a good idea to create tickets for the affected libraries with localized patches instead of posting here cumulative patches to the whole Boost. This way the library maintainers will be properly notified of the planned changes, the changes won't be lost and will be applied when it is convenient.
That's not practical. I'm changing many libraries, and I'm making micro-commits for easy review. I'm not going to file a ticket for each one. If you want to know how your library will be affected by my changes, then simply grep for the macros under discussion for removal (BOOST_NO_SFINAE, at least, currently).
The thing is that changes to particular libraries get lost in the cumulative patches posted to the list. Not everyone is reading all posts to the list. I'm not insisting, though, it was just a suggestion.
On 10/13/2013 02:26 PM, Stephen Kelly wrote:
On 10/13/2013 02:17 PM, John Maddock wrote:
The attached patch removes support for those versions of the compilers which are not capable of SFINAE.
This bumps the compiler requirement to remove support for Borland 5.x, Metrowerks prior to 9.0, and SunPro prior to 5.7. Compilers based on EDG 238 are also affected, though it is unknown what those are.
Ok to commit?
Please, wait for at least John give you permission.
It's OK by me, though as others have already said, best to wait for the 1.55 release to go out first... just in case we need to test patches/bug reports in Trunk.
This is what I don't understand.
I don't see how my work makes this any harder. At *worst*, a bug fix will have a part in the positive side of an ifdef for some feature (like BOOST_NO_SFINAE), and a part in the negative side of it. I don't think that's even a likely scenario. Also, that's assuming the one who writes the patch patches both sides of the ifdef.
That is still trivial to cherry-plck into the trunk branch. Or do you think it is not trivial? In that case, what makes it non-trivial?
I want to understand the problem that exists and is stalling me.
I do not believe there are that many people questioning the work you are doing. The problem is that until 1.55 is officially out, it is possible that some change in trunk will still be merged to release and that change might involve some of the changes you are making, leading to a much more complicated merge. That is why I believe you were asked to hold off on your changes to trunk until after 1.55 has been released. In that sense your work makes things harder at this moment if a quick late merge has to be done. Afterward I am sure your work will make things easier for many libraries, but it still is important for library developers to know what you have done.
I don't see how my work makes this any harder. At *worst*, a bug fix will have a part in the positive side of an ifdef for some feature (like BOOST_NO_SFINAE), and a part in the negative side of it. I don't think that's even a likely scenario. Also, that's assuming the one who writes the patch patches both sides of the ifdef.
That is still trivial to cherry-plck into the trunk branch. Or do you think it is not trivial? In that case, what makes it non-trivial?
I want to understand the problem that exists and is stalling me.
If person A makes a commit that breaks the tests from running, person B can't see if his changes are working or not. So high risk changes to common code in Trunk should be discouraged during the release period. John. PS, I'm not saying you will break anything, just that with the number of changes going through.... chances are you're bound to... I would!
participants (6)
-
Andrey Semashev -
Edward Diener -
John Maddock -
Stephen Kelly -
Vicente J. Botet Escriba -
Václav Zeman