Modernize macros to use `do { } while (0)` instead of `{ }`

[terrelln]

Our macros don't currently use the safer do { } while (0) construction because our Visual Studios CI used to error on this construction. It complained about constant false branches. Our new Visual Studios CI doesn't error on do { } while (0).

terrelln#4

This is standards compliant behavior, and newer Visual Studios doesn't warn on it, so IMO we should switch to this construction.

Based on this discussion this bug was fixed in VS2015. So the tradeoff is that we will get warnings/errors with Visual Studios older than 2015.

[Cyan4973]

Yes, Visual Studio has been the main reason to not employ while (0).

Which minimal version of Visual Studio is required to avoid triggering a "constant branch" warning ?

[terrelln]

Which minimal version of Visual Studio is required to avoid triggering a "constant branch" warning ?

I'm not really sure. I tried to trigger it in godbolt, but I don't think they have a old enough version.

[terrelln]

Based on this discussion VS2012 does emit this warning.

[terrelln]

And in the same discussion, looks like it was fixed in VS2015.

[Cyan4973]

While CI seems safe, by virtue of supporting only fairly recent versions of Visual Studio,
this decision may have consequences on https://github.com/facebook/zstd/tree/dev/build,
since we are supporting Visual Studio since 2008 here.

Depending on how we want to handle that, we might have to drop a few versions.
Your investigation suggests that versions <= 2012 would trigger a warning.
So I think that leaves only one version, VS2013, as a question mark.

[terrelln]

I suspect V2013 does have the bug, since it was mentioned it was fixed in 2015.

[Cyan4973]

I wonder if there would be a way to disable this warning from within the project file,
or possibly even better, from within the cmake script that will generate the project file.

[terrelln]

I don't really know what our policy should be here. We will still compile with older Visual Studios, it will just emit warnings. So our users would have to turn off warnings-as-errors for older Visual Studios.

We should be able to disable the warning using __pragma(warning(suppress:4127)). So we could probably push that pragma in our portability header, and then use do { } while (0) without generating those warnings.

[terrelln]

It turns out we already have a few macros that use do { } while (0) in our error_private.h header. And we've made a release and had no complaints. So we should be able to add it no problem.