Compiler warnings #370

New Issue

claunia · 2026-01-29T20:42:49Z

claunia commented

2026-01-29 20:42:49 +00:00

Originally created by @crazydef on GitHub (Jan 5, 2021).

Hi,

When including Brotli headers in our own projects, we're experiencing the following compiler warnings in Visual Studio 2019:

brotli\dec\bit_reader.h(27,47): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(110,74): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(119,75): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(108,41): warning C4100: 'n_bits': unreferenced formal parameter
brotli\dec\bit_reader.h(252,39): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(272,39): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(307,39): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(27,47): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(110,74): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(119,75): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(108,41): warning C4100: 'n_bits': unreferenced formal parameter
brotli\dec\bit_reader.h(252,39): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(272,39): warning C4127: conditional expression is constant
brotli\dec\bit_reader.h(307,39): warning C4127: conditional expression is constant
brotli\dec\huffman.c(124,1): warning C4127: conditional expression is constant
brotli\dec\huffman.c(135,1): warning C4127: conditional expression is constant
brotli\dec\decode.c(1174,7): warning C4127: conditional expression is constant

It'd be nice if we didn't see these in the future. :)

Originally created by @crazydef on GitHub (Jan 5, 2021). Hi, When including Brotli headers in our own projects, we're experiencing the following compiler warnings in Visual Studio 2019: brotli\dec\bit_reader.h(27,47): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(110,74): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(119,75): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(108,41): warning C4100: 'n_bits': unreferenced formal parameter brotli\dec\bit_reader.h(252,39): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(272,39): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(307,39): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(27,47): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(110,74): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(119,75): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(108,41): warning C4100: 'n_bits': unreferenced formal parameter brotli\dec\bit_reader.h(252,39): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(272,39): warning C4127: conditional expression is constant brotli\dec\bit_reader.h(307,39): warning C4127: conditional expression is constant brotli\dec\huffman.c(124,1): warning C4127: conditional expression is constant brotli\dec\huffman.c(135,1): warning C4127: conditional expression is constant brotli\dec\decode.c(1174,7): warning C4127: conditional expression is constant It'd be nice if we didn't see these in the future. :)

claunia closed this issue

2026-01-29 20:42:50 +00:00

claunia commented

2026-01-29 20:42:50 +00:00

@eustas commented on GitHub (Jan 18, 2021):

brotli\dec\bit_reader.h(108,41): warning C4100: 'n_bits': unreferenced formal parameter could be fixed.

Others are just compiler suspicions - it is explicit that we expect "compile-time-constant" conditional expression there, to gain performance on different platforms / targets. So, you should consider suppressing warning 4127 via command line parameters.

@eustas commented on GitHub (Jan 18, 2021): `brotli\dec\bit_reader.h(108,41): warning C4100: 'n_bits': unreferenced formal parameter` could be fixed. Others are just compiler suspicions - it is explicit that we expect "compile-time-constant" conditional expression there, to gain performance on different platforms / targets. So, you should consider suppressing warning 4127 via command line parameters.

claunia commented

2026-01-29 20:42:50 +00:00

@crazydef commented on GitHub (Jan 18, 2021):

If they're expected, you should be suppressing them.

Taking the following as an example, it would be better (in my opinion) to do something like this anyway:

static BROTLI_INLINE uint32_t BitMask(uint32_t n)
{
#if BROTLI_HAS_UBFX
  /* Masking with this expression turns to a single
     "Unsigned Bit Field Extract" UBFX instruction on ARM. */
  return ~((0xFFFFFFFFu) << n);
#elif BROTLI_HAS_IS_CONSTANT
  if (BROTLI_IS_CONSTANT(n))
  {
    return ~((0xFFFFFFFFu) << n);
  }
#endif
  return kBrotliBitMask[n];
}

We're not so relaxed about suppressing compiler warnings in our code and it would be nice to not have to constantly play catch up when someone includes your headers in a new part of our code.

@crazydef commented on GitHub (Jan 18, 2021): If they're expected, you should be suppressing them. Taking the following as an example, it would be better (in my opinion) to do something like this anyway: ``` static BROTLI_INLINE uint32_t BitMask(uint32_t n) { #if BROTLI_HAS_UBFX /* Masking with this expression turns to a single "Unsigned Bit Field Extract" UBFX instruction on ARM. */ return ~((0xFFFFFFFFu) << n); #elif BROTLI_HAS_IS_CONSTANT if (BROTLI_IS_CONSTANT(n)) { return ~((0xFFFFFFFFu) << n); } #endif return kBrotliBitMask[n]; } ``` We're not so relaxed about suppressing compiler warnings in our code and it would be nice to not have to constantly play catch up when someone includes your headers in a new part of our code.

claunia commented

2026-01-29 20:42:51 +00:00

@eustas commented on GitHub (Jan 18, 2021):

We also do care about code quality and eliminate warnings.

But here problem is not with BROTLI_IS_CONSTANT macro; it is about compiler being picky.
When MSVC adds support for __builtin_constant_p the code snippet you have provided will also generate warning, because BROTLI_IS_CONSTANT(x) has (instance-wise) constant value.

If we look at warning in huffman.c - compiler does not like how BROTLI_REPEAT macro works:

#define BROTLI_REPEAT(N, X) {     \
  if ((N & 1) != 0) {X;}          \
  if ((N & 2) != 0) {X; X;}       \
  if ((N & 4) != 0) {X; X; X; X;} \
}

I hope we would be able to find a solution that will calm down MSVC...

@eustas commented on GitHub (Jan 18, 2021): We also do care about code quality and eliminate warnings. But here problem is not with `BROTLI_IS_CONSTANT` macro; it is about compiler being picky. When MSVC adds support for `__builtin_constant_p` the code snippet you have provided will also generate warning, because `BROTLI_IS_CONSTANT(x)` has (instance-wise) constant value. If we look at warning in `huffman.c` - compiler does not like how `BROTLI_REPEAT` macro works: ``` #define BROTLI_REPEAT(N, X) { \ if ((N & 1) != 0) {X;} \ if ((N & 2) != 0) {X; X;} \ if ((N & 4) != 0) {X; X; X; X;} \ } ``` I hope we would be able to find a solution that will calm down MSVC...

claunia commented

2026-01-29 20:42:51 +00:00

@crazydef commented on GitHub (Jan 18, 2021):

Microsoft will almost definitely never add support for that. The Visual Studio compiler is a C++ compiler that happens to have some limited support for older versions of C. In C++ there are better, standard ways to do this.

@crazydef commented on GitHub (Jan 18, 2021): Microsoft will almost definitely never add support for that. The Visual Studio compiler is a C++ compiler that happens to have some limited support for older versions of C. In C++ there are better, standard ways to do this.

claunia commented

2026-01-29 20:42:51 +00:00

@eustas commented on GitHub (Jan 3, 2023):

BROTLI_REPEAT was unwrapped. Will check soon what remaining warnings we have and fix them.

@eustas commented on GitHub (Jan 3, 2023): `BROTLI_REPEAT` was unwrapped. Will check soon what remaining warnings we have and fix them.

claunia referenced this issue

2026-01-29 20:51:52 +00:00

[PR #370] [MERGED] Convert encoder to plain C. #967

claunia referenced this issue

2026-01-29 20:51:53 +00:00

[PR #370] Convert encoder to plain C. #970

Sign in to join this conversation.