`<typeinfo>` unconditionally included on dinkumware

[Kojoley]

It was added in 668b3fccaef7460036a1a83a9c90df8cbe76c4ab to fix lexical_cast (https://svn.boost.org/trac10/ticket/4115) problem, but that seems to be an overkill (and probably not needed since https://github.com/boostorg/lexical_cast/commit/3ce36a28484b76fab2dd96e8ee43f52ae87d1353?). Also, it is a strange thing that with defined BOOST_NO_STD_TYPEINFO there is std::typeinfo. It completely undermines the purpose of the macro and of boost/core/typeinfo.hpp.

[Kojoley]

I have reproduced the https://svn.boost.org/trac10/ticket/4115 problem by applying https://github.com/boostorg/lexical_cast/pull/31 and https://github.com/boostorg/config/pull/307 on VC9.0/10.0/11.0, the fix for LexicalCast proposed in https://github.com/boostorg/lexical_cast/pull/32.

[jzmaddock]

I'm curious to know why you think injecting typeinfo into the namespace in which it really should be defined anyway is such a bad idea? Though we should certainly restrict the workaround to pre-msvc-12, as it's only required for obsolete compilers anyway.

[Kojoley]

I did not say it is a bad idea, it is convenient but costly.

[jeremy-murphy]

The IAR compiler for Arm uses Dinkumware and if I disable exceptions, which is admittedly I guess a non-standard extension, then it fails to compile this injection because it does not have ::typeinfo. So, I would be in favour of limiting this kind of thing to only the compilers where it is known to work, rather than assuming it works in general.

[jeremy-murphy]

Digging around more on this, I'm not sure that #307 is the right solution after all.

I fixed this another way by a very simple modification to dinkumware.hpp on line 89, which currently is:

#if BOOST_MSVC < 1800

but I think it should be:

#if defined(_MSC_VER) && BOOST_MSVC < 1800

What do you think, @jzmaddock? I believe you added that line 6 months ago. :)

[jzmaddock]

You're correct: that's supposed to detect old MSVC versions, checking for defined(BOOST_MSVC) should do the trick also?

[Kojoley]

#307 is the right way, because Boost.TypeInfo is a portable solution to the problem the code is solving.

[Kojoley]

You're correct: that's supposed to detect old MSVC versions, checking for defined(BOOST_MSVC) should do the trick also?

defined(__ghs__) is clearly detecting something different than MSVC

[glenfe]

You're correct: that's supposed to detect old MSVC versions, checking for defined(BOOST_MSVC) should do the trick also?

Yes to defined(BOOST_MSVC) since the whole point of BOOST_MSVC is avoid checking _MSC_VER which MSVC-pretenders define.

[jeremy-murphy]

You're correct: that's supposed to detect old MSVC versions, checking for defined(BOOST_MSVC) should do the trick also?

Sure, I was just suggesting what I already saw used repeatedly within that file, but now I realize that they were testing the version of _MSC_VER whereas this is BOOST_MSVC. I trust you all know these macros better than I do.

[jeremy-murphy]

Even though we might agree about that line 89 needing a defined(BOOST_MSVC), it is actually tangential to this issue, which is about unconditionally including <typeinfo>, when there is an existing cross-library solution for it in Boost.Core.

I'm inclined to agree with @Kojoley that their solution is the right way to go, but I think the PR needs to also remove all the references to BOOST_NO_STD_TYPEINFO from the tests, etc? Since I think Dinkumware is the only place that defines it.

So, I would be super happy about someone putting the defined(BOOST_MSVC) in line 89 in the interim, to at least fix the broken conditional.

[GregKon]

Even though we might agree about that line 89 needing a defined(BOOST_MSVC), it is actually tangential to this issue, which is about unconditionally including <typeinfo>, when there is an existing cross-library solution for it in Boost.Core.

I'm inclined to agree with @Kojoley that their solution is the right way to go, but I think the PR needs to also remove all the references to BOOST_NO_STD_TYPEINFO from the tests, etc? Since I think Dinkumware is the only place that defines it.

So, I would be super happy about someone putting the defined(BOOST_MSVC) in line 89 in the interim, to at least fix the broken conditional.

I does confirm problem (IAR) and possibly solution works.