In-Reply-To: <dh1gjf$bt0$1@sea.gmane.org> kalita@poczta.onet.pl (Marcin Kalicinski) wrote (abridged):

On the other hand some people (including me) consider __assume to be nice, helping optimization and suppressing annoying warnings.

Including me, too. I don't think anyone has spoken against __assume in principle. The issue has been how best to incorporate it.

Put __assume in BOOST_ASSUME and leave BOOST_ASSERT unmodified.

This to me is the safest option.

Pros: - does not disturb existing code in any way

Cons: - no existing code uses it, i.e. all existing libraries would have to be modified to take advantage of it; this will probably never happen so most of the code will not benefit

__assume is a performance optimisation. As such I think it is fine for it to be adopted primarily in places where someone has seen the need (and in new code). It doesn't need to be adopted in /all/ libraries. If no-one thinks __assume is valuable enough to be worth changing old code to use it, perhaps it is not worth adding to boost at all. Is it entirely a Microsoft thing, or would some other compilers benefit too?

- obscure: most of the people would be confused about it when seeing it for the first time

Surely no more so than any other boost library? I'd have thought it would be even more confusing if BOOST_ASSERT changed its meaning. At least the people who don't know what ASSUME means know they don't know, and can look it up. A change to ASSERT will slip under many people's guard. BOOST_ASSERT and BOOST_ASSUME have different semantics. I think it's good to let authors express which semantics they intended.

2. BOOST_ASSERT + define to enable assume (default: on) Cons:

- breaks user code which uses BOOST_ASSERT in "uncommon" way.

And boost code as well. Probably there is no such code in boost, but I don't think anyone has gone through it all to check. Perhaps no user code would be broken either. To me this really feels like a gambler's option. Silently breaking users code is unacceptable, but maybe we are feeling lucky tonight. Or maybe we just don't care, and can rule such code unsupported.

BOOST_ASSERT + define to enable assume (default: off) - does enable assume on all the boost code, some user programs get smaller and faster with a simple -D"BOOST_ASSERT_USE_ASSUME" on command line.

In order to switch it on safely they need to know that every use of BOOST_ASSERT in every module they link, and every header they #include, is safe with the new semantics. I don't see how they can know this without laboriously checking every place where BOOST_ASSERT is used and thinking hard about the semantics of each one. This has to be repeated for every new release of the libraries. It's a ton of work, repeated once for each user. To me it makes far more sense for the work to be done once by the author of the code that uses the assert, and for the result of that analysis to be recorded in the code for all time by renaming from BOOST_ASSERT to BOOST_ASSUME. This will also help anyone reading the code. Another drawback of the global flag is that if just one place turns out to require the old semantics, no other places can use the new semantics. The whole app suffers. (It'd be tempted to define BOOST_ASSERT_USE_ASSUME for some modules and not others, but that leads to violation of the One Definition Rule.) In practice I think this option is the same as the previous one. Both are saying the "uncommon" uses are not supported any more. Certainly they can't be used within any library code, because the author doesn't know what options the user will compile it with. I don't think we want to double the testing burden by testing both with and without -D"BOOST_ASSERT_USE_ASSUME". The effect of -D"BOOST_ASSERT_USE_ASSUME" is to inject undefined behaviour into the code. To me that is huge. It's not a minor change of semantics. -- Dave Harris, Nottingham, UK.