build: Disable Visual Studio warning "conditional expression is const…

[jay]

…ant"

• Disable warning C4127 "conditional expression is constant" when
  building with Microsoft's compiler.

C4127 was already disabled in the limited circumstance of the WHILE_FALSE
macro which disabled the warning specifically for while(0). This commit
removes the WHILE_FALSE macro in favor of disabling C4127 everywhere.

We have various macros that cause 0 or 1 to be evaluated, which causes
warning C4127 in Visual Studio. For example this causes it:

#define Curl_resolver_asynch() 1

Full behavior is not clearly defined and inconsistent across versions.
However it is documented that since VS 2015 Update 3 Microsoft has
addressed this somewhat but not entirely, not warning on while(true) for
example.

Prior to this change some C4127 warnings occurred when I built with
Visual Studio.

Closes #xxxx

---

/cc @danielgustafsson

[danielgustafsson]

I don't know Visual Studio so I might be off, but won't this risk that code which should've invoked an actual warning now passes?

[jay]

I don't know Visual Studio so I might be off, but won't this risk that code which should've invoked an actual warning now passes?

Yeah but that's the point. In my opinion the warning is a nuisance and only VS does this. I can't recall ever seeing a conditional constant expression that actually was a mistake.

[MarcelRaad]

I've actually seen C4127 discover a bug once. IIRC, it was a comparison which should have been an assignment and the variable was wrongly "fixed" to be const instead of assigned to. Nothing that should ever have passed code review though, so I'm fine with disabling it.

One thing though, it's disabled for the CMake build in

curl/lib/CMakeLists.txt

Line 23 in 6932849

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /wd4127")

. That could then be removed.

[jay]

One thing though, it's disabled for the CMake build in

Thanks. I didn't realize that prior to this change 4127 was already disabled in the other build systems that can use microsoft's compiler, winbuild and cmake. Since this commit adds a single disable in curl_setup.h that should work for all build systems. I've now removed the others (see the squashme).

[danielgustafsson]

On 30 Nov 2019, at 23:40, Jay Satiro ***@***.***> wrote: One thing though, it's disabled for the CMake build in Thanks. I didn't realize that prior to this change 4127 was already disabled in the other build systems that can use microsoft's compiler, winbuild and cmake. Since this commit adds a single disable in curl_setup.h that should work for all build systems. I've now removed the others (see the squashme).

Thanks for the extensive discussion, very heloful for those of us without MSVC experience. +1 on going ahead with this.