Resolve visual studio warning C4127: conditional expression is constant.

[dgeelen]

This resolves a warning in Visual Studio which is currently prevents using boost::iostreams::stream<> when this warning is enabled.

What seems to happen is that the first part of the expression is recognized as constant, but somehow the second part is not taken into account to see that the expression as a whole is not constant. Flipping the two around fixes the warning, but not the problem. In principle this code should still trigger the warning, because you probably want to know when a sub-expression in an if() statement is constant too. Because a constant sub-expression seems just as wrong a a constant full expression.

However, this seems to solve the warning without any changes to the logic, so this is a simple fix for now.

[codecov[bot]]

Codecov Report

Merging #118 into develop will not change coverage. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #118   +/-   ##
========================================
  Coverage    68.84%   68.84%           
========================================
  Files           80       80           
  Lines         3444     3444           
  Branches      1027     1027           
========================================
  Hits          2371     2371           
  Misses         454      454           
  Partials       619      619           

Impacted Files	Coverage Δ
.../iostreams/detail/streambuf/indirect_streambuf.hpp	75.30% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bc86890...726cbe1. Read the comment docs.

[dgeelen-uipath]

So, what's happening here? Is there an actual problem or can this be merged? It seems to me that this a false positive in the codecov report? @mclow

[dgeelen-uipath]

ping @Lastique @mclow @jeking3 @pdimov

[Lastique]

1. I'm not the maintainer, and I cannot merge this.
2. I wouldn't merge because I would prefer to disable the warning with a pragma rather than modifying the condition. We actually want the conditional expression to be constant.

[dgeelen-uipath]

2. I wouldn't merge because I would prefer to disable the warning with a pragma rather than modifying the condition. We actually want the conditional expression to be constant.

But I didn't change that, the condition is still constant (and exactly the same condition as before). All I did was to swap the left and right-hand side of the equality. This is sufficient to suppress the warning in Visual Studio, as it seems to only scan for 'unconditional constantness'. I.e. a constant after a run-time check is not considered constant.

Given that I didn't make any changes to the actual logic, what are your thoughts on the CodeCov report?

[Lastique]

All I did was to swap the left and right-hand side of the equality.

You changed the order of operands of operator||, which makes it non-constant. Which is probably why the compiler stopped complaining. See logical operators short-circuiting.

[dgeelen-uipath]

All I did was to swap the left and right-hand side of the equality.

You changed the order of operands of operator||, which makes it non-constant. Which is probably why the compiler stopped complaining. See logical operators short-circuiting.

I didn't really see that as a problem, but if you're worried about e.g. performance than I'm also fine with disabling it through a #pragma. It's just that I preferred to resolve the problem without relying on compiler specifics.

Please see my updated commit for the #pragma version.

[Lastique]

The pragmas must be guarded with defined(BOOST_MSVC), for which boost/config.hpp must be included. Or better yet, disable/restore the warning globally in boost/iostreams/detail/config/disable_warnings.hpp and boost/iostreams/detail/config/enable_warnings.hpp.

Actually, I can see that this warning is already disabled there. So I'm not sure why you're having this problem. What is your compiler? Does the problem reproduce with the current develop branch?

[dgeelen-uipath]

I've added the guards around the offending code.

I don't quite get how MSVC parses this, but perhaps it has something to do with the fact that this code is inside a template? If I don't put the #pragma inside of the template (near the offending code), it doesn't seem to want to disable. Even if I put it after the #include at the end that pops the warning state it will still trigger. Although maybe some other header somewhere is turning it on again, as I can disable by putting the #pragma just after the #include <boost/iostreams/stream.hpp> in my code.

This is with Visual Studio Enterprise 2019 Preview - Version 16.7.0 Preview 5.0 / Microsoft (R) C/C++ Optimizing Compiler Version 19.27.29109 for x64. I don't think there is any real difference between 1.73 and develop for this header.

[Lastique]

I'd recommend reporting this to Microsoft as it doesn't look like a correct behavior of the pragma to me.

[pdimov]

I see nothing wrong with disabling the warning locally like this.

[Lastique]

It shouldn't be needed as it is already disabled.

Though as a workaround for a compiler bug it's probably fine. I would also add a comment explaining why this workaround is needed.