"Remove obsolete MSVC checks" and _MSC_VER
One general note about these changes. This: #if defined( _MSC_VER ) && _MSC_VER >= 1310 doesn't mean "compiler is MS VC++ 7.1". It means "compiler claims to support MSVC 7.1's features." Many Windows compilers define _MSC_VER for compatibility. I'm not positive that the removal of the _MSC_VER >= part would break something nowadays, but if it does, this would be why. I'd have left these alone; it's not clear what purpose removing the check is intended to serve, apart from breaking code. Actual MSVC-specific code is guarded by BOOST_MSVC and not _MSC_VER.
On 09/26/2013 06:41 PM, Peter Dimov wrote:
One general note about these changes. This:
#if defined( _MSC_VER ) && _MSC_VER >= 1310
doesn't mean "compiler is MS VC++ 7.1". It means "compiler claims to support MSVC 7.1's features."
From the point of view of the source code, those are the same.
Does that make sense? The source code uses features which MSVC 7.1 has, and assumes not to need to use ifdefs guarding such code. That is also true for new source code. People writing new source code are not going to check if it worked with all compilers pretending to be MSVC 7.1 (but not actually MSVC 7.1) and wrap it in an ifdef. We bumped the compiler requirement so that they don't have to do that. If another compiler reports that it has the features of MSVC 7.1, then that is truth, and the source code should not assume it is false.
Many Windows compilers define _MSC_VER for compatibility.
I'm not positive that the removal of the _MSC_VER >= part would break something nowadays, but if it does, this would be why. I'd have left these alone; it's not clear what purpose removing the check is intended to serve, apart from breaking code.
* Removal of near-duplicate code * Removal of obsolete workarounds (pulling in dependencies), * Removal of noise in git grep output (so you can see if a feature/define is actually used, or if it is only used inside an obsolete ifdef. git grep does not know about ifdefs) * Making the code look more like normal c++ (eg revisions 85957 and 85958), which makes reading/contributing easier. * Making it easier to move code around for modularization. ie, the purpose is the same purpose as justified the compiler requirement bump.
Actual MSVC-specific code is guarded by BOOST_MSVC and not _MSC_VER.
Only if written correctly, which is not always the case. See revision 85932, which specifically references MSVC warnings. This is moot anyway because of the above. Thanks, Steve.
Stephen Kelly wrote:
People writing new source code are not going to check if it worked with all compilers pretending to be MSVC 7.1 (but not actually MSVC 7.1) and wrap it in an ifdef. We bumped the compiler requirement so that they don't have to do that.
I don't understand. We bumped the compiler requirements for MSVC. We did not bump the compiler requirements for all other compilers that define _MSC_VER. Perhaps some of them define it to 1200 or 1300. Not emulating MSVC 7.1 features is not a reason to disqualify a compiler.
* Removal of near-duplicate code * Removal of obsolete workarounds (pulling in dependencies),
These do not apply to the cases I have in mind, which use _MSC_VER properly and not incorrectly, and in which the only change is from #if defined( _MSC_VER ) && _MSC_VER >= ... to #if defined( _MSC_VER ) Nothing is removed, nothing is obsolete, nothing is simplified. I see no upside here.
On 09/26/2013 07:13 PM, Peter Dimov wrote:
Stephen Kelly wrote:
People writing new source code are not going to check if it worked with all compilers pretending to be MSVC 7.1 (but not actually MSVC 7.1) and wrap it in an ifdef. We bumped the compiler requirement so that they don't have to do that.
I don't understand.
We bumped the compiler requirements for MSVC. We did not bump the compiler requirements for all other compilers that define _MSC_VER.
Perhaps some of them define it to 1200 or 1300. Not emulating MSVC 7.1 features is not a reason to disqualify a compiler.
Why not? I think it is. If a compiler claims it only supports MSVC6 features (1200), then all use of MSVC > 6 features need to be wrapped in ifdefs, and I can revert everything I've committed so far, right? I guess I don't understand your position.
* Removal of near-duplicate code * Removal of obsolete workarounds (pulling in dependencies),
These do not apply to the cases I have in mind, which use _MSC_VER properly and not incorrectly, and in which the only change is from
#if defined( _MSC_VER ) && _MSC_VER >= ...
to
#if defined( _MSC_VER )
Nothing is removed, nothing is obsolete, nothing is simplified. I see no upside here.
Ok, I see your logic here. Nothing is removed, except, in this case, part of an #if logic. The code block inside remains. However, I don't understand your position: I don't understand why such _MSC_VER usage should be put back in. Maybe if you can get concrete and point out a compiler in config/compiler/* that could have a problem with the above kind _MSC_VER removal, then I might understand. However, if such a compiler is ancient itself, it makes sense to bump the version of that one too for the same reasons as before. Are you saying that you think such _MSC_VER usage should never be removed? Thanks, Steve.
Stephen Kelly wrote:
If a compiler claims it only supports MSVC6 features (1200), then all use of MSVC > 6 features need to be wrapped in ifdefs, and I can revert everything I've committed so far, right?
Of course not. This compiler is not MSVC6 and BOOST_MSVC is not defined for it. No MSVC workarounds are applied (unless by mistake).
Maybe if you can get concrete and point out a compiler in config/compiler/* that could have a problem with the above kind _MSC_VER removal, then I might understand.
I can't. I have no idea what compilers in use today (besides Intel) define _MSC_VER and to what.
Stephen Kelly wrote:
If a compiler claims it only supports MSVC6 features (1200), then all use of MSVC > 6 features need to be wrapped in ifdefs, and I can revert everything I've committed so far, right?
Of course not. This compiler is not MSVC6 and BOOST_MSVC is not defined for it. No MSVC workarounds are applied (unless by mistake).
To clarify, by "features" I mean MSVC-specific extensions, and not standard C++ features. The intrinsic _ReadWriteBarrier, for example, first appeared in MSVC 7.1. A compiler that defines _MSC_VER to 1300 is unlikely to support it, even if it supports the whole of C++98 or C++03.
On 09/26/2013 07:47 PM, Peter Dimov wrote:
The intrinsic _ReadWriteBarrier, for example, first appeared in MSVC 7.1. A compiler that defines _MSC_VER to 1300 is unlikely to support it,
Did you mean to say 'defines _MSC_VER to 1310' ? If a compiler defines _MSC_VER to 1300, that is MSVC 7.0 compatibility, so not supporting a 7.1 feature is not unexpected. However, if the compiler says it is 7.1 compatible (1310), why is it unlikely to support ReadWriteBarrier (a 7.1 feature, so part of 7.1 compatibility)? Thanks, Steve.
On 09/26/2013 07:38 PM, Peter Dimov wrote:
Maybe if you can get concrete and point out a compiler in config/compiler/* that could have a problem with the above kind _MSC_VER removal, then I might understand.
I can't. I have no idea what compilers in use today (besides Intel) define _MSC_VER and to what.
Ok, I think the best way forward is to find out what compiler versions no longer should be supported, and finding out about the msvc compatibility of the remaining minimum version. That means bringing this thread to a conclusion: http://thread.gmane.org/gmane.comp.lib.boost.devel/243923/focus=244226 At least Borland and codegear have a msvc mode, according to my grep in config/compilers. Comeau does too, but I have no idea if that one has a successor or if it is so ancient it should be dropped. Any clue? What can you tell us about intel? Thanks, Steve.
Le 26/09/13 22:43, Stephen Kelly a écrit :
Maybe if you can get concrete and point out a compiler in config/compiler/* that could have a problem with the above kind _MSC_VER removal, then I might understand. I can't. I have no idea what compilers in use today (besides Intel) define _MSC_VER and to what. Ok, I think the best way forward is to find out what compiler versions no longer should be supported, and finding out about the msvc compatibility of the remaining minimum version. That means bringing this
On 09/26/2013 07:38 PM, Peter Dimov wrote: thread to a conclusion:
http://thread.gmane.org/gmane.comp.lib.boost.devel/243923/focus=244226
Hi, IMO this thread conclusion was to don't change anything. In any case you should have the acknowledge of all the authors of the libraries you have changed. Vicente
On 09/26/2013 10:51 PM, Vicente J. Botet Escriba wrote:
Le 26/09/13 22:43, Stephen Kelly a écrit :
Maybe if you can get concrete and point out a compiler in config/compiler/* that could have a problem with the above kind _MSC_VER removal, then I might understand. I can't. I have no idea what compilers in use today (besides Intel) define _MSC_VER and to what. Ok, I think the best way forward is to find out what compiler versions no longer should be supported, and finding out about the msvc compatibility of the remaining minimum version. That means bringing this
On 09/26/2013 07:38 PM, Peter Dimov wrote: thread to a conclusion:
http://thread.gmane.org/gmane.comp.lib.boost.devel/243923/focus=244226
Hi,
IMO this thread conclusion was to don't change anything. In any case you should have the acknowledge of all the authors of the libraries you have changed.
I'm sorry, what are you referring to? Are you referring to the mails that appeared in the last few minutes? Thanks, Steve.
Le 26/09/13 22:54, Stephen Kelly a écrit :
Le 26/09/13 22:43, Stephen Kelly a écrit :
Maybe if you can get concrete and point out a compiler in config/compiler/* that could have a problem with the above kind _MSC_VER removal, then I might understand. I can't. I have no idea what compilers in use today (besides Intel) define _MSC_VER and to what. Ok, I think the best way forward is to find out what compiler versions no longer should be supported, and finding out about the msvc compatibility of the remaining minimum version. That means bringing this
On 09/26/2013 07:38 PM, Peter Dimov wrote: thread to a conclusion:
http://thread.gmane.org/gmane.comp.lib.boost.devel/243923/focus=244226
Hi,
IMO this thread conclusion was to don't change anything. In any case you should have the acknowledge of all the authors of the libraries you have changed. I'm sorry, what are you referring to? Are you referring to the mails
On 09/26/2013 10:51 PM, Vicente J. Botet Escriba wrote: that appeared in the last few minutes?
There was a sentence that me the mail you sent yesterday "I'll commit theses parts of the patch tomorrow unless there are objections before then." This was the 25th and today there are more than 20 commits all around the Boost code. You can not pretend that the authors will replay you one day later. Maybe you use to do these kind of changes in other projects, but in Boost we try to discuss a little bit more. Best, Vicente
On 09/26/2013 11:09 PM, Vicente J. Botet Escriba wrote:
Le 26/09/13 22:54, Stephen Kelly a écrit :
Le 26/09/13 22:43, Stephen Kelly a écrit :
Maybe if you can get concrete and point out a compiler in config/compiler/* that could have a problem with the above kind _MSC_VER removal, then I might understand. I can't. I have no idea what compilers in use today (besides Intel) define _MSC_VER and to what. Ok, I think the best way forward is to find out what compiler versions no longer should be supported, and finding out about the msvc compatibility of the remaining minimum version. That means bringing
On 09/26/2013 07:38 PM, Peter Dimov wrote: this thread to a conclusion:
http://thread.gmane.org/gmane.comp.lib.boost.devel/243923/focus=244226
Hi,
IMO this thread conclusion was to don't change anything. In any case you should have the acknowledge of all the authors of the libraries you have changed. I'm sorry, what are you referring to? Are you referring to the mails
On 09/26/2013 10:51 PM, Vicente J. Botet Escriba wrote: that appeared in the last few minutes?
There was a sentence that me the mail you sent yesterday
"I'll commit theses parts of the patch tomorrow unless there are objections before then."
Hello, There certainly seems to be some confusion. Here is the mail you are referring to: http://thread.gmane.org/gmane.comp.lib.boost.devel/243923/focus=244226 I sent it less than an hour ago, not yesterday. Thanks, Steve.
participants (3)
-
Peter Dimov -
Stephen Kelly -
Vicente J. Botet Escriba