glsl-optimizer spews warnings about invalid C++

[froydnj]

(Filing here because issues on glsl-optimizer are closed.)

This code:

https://github.com/jamienicol/glsl-optimizer/blob/a9bdfcc8d80050e45e40d9cf806f3e38f339afd0/src/compiler/glsl/ir.h#L139-L174

spews warnings -- maybe just with sccache-dist -- because assume(this != NULL) is trivially true:

warning: glsl-optimizer/src/compiler/glsl/ir.h:159:203: warning: 'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to true [-Wtautological-undefined-compare]
warning:    class ir_rvalue *as_rvalue() { do { ((this != __null) ? static_cast<void> (0) : __assert_fail ("this != __null", "glsl-optimizer/src/compiler/glsl/ir.h", 159, __PRETTY_FUNCTION__)); __builtin_assume(this != __null); } while (0); return is_rvalue() ? (ir_rvalue *) this : __null; } const class ir_rvalue *as_rvalue() const { do { ((this != __null) ? static_cast<void> (0) : __assert_fail ("this != __null", "glsl-optimizer/src/compiler/glsl/ir.h", 159, __PRETTY_FUNCTION__)); __builtin_assume(this != __null); } while (0); return is_rvalue() ? (ir_rvalue *) this : __null; }

It'd be nice if compiling the optimizer wasn't generating voluminous output.

[jamienicol]

Which compiler and version are you using? Is this part of a gecko build, or standalone. In either case the env would be useful.

I can't see this warning on a local build, but I don't have sccache-dist set up. I do however see another warning:

warning: glsl-optimizer/src/util/softfloat.c:214:42: warning: result of comparison of constant 32768 with expression of type 'int16_t' (aka 'short') is always false [-Wtautological-constant-out-of-range-compare]
warning:         } else if ((e > 0x1d) || (0x8000 <= m)) {
warning:                                   ~~~~~~ ^  ~
warning: 1 warning generated.

I'm hesitant to make fixes to glsl-optimizer, as it is based on mesa's source code and we want to be able to rebase it as easily as possible. My build script sets .warnings(false) on the cc::Build object, which I hoped would be enough to silence warnings. Perhaps this is a bug in cc?

(Filing here because issues on glsl-optimizer are closed.)

Not sure why that was the case, but I have enabled issues now.

[froydnj]

Which compiler and version are you using? Is this part of a gecko build, or standalone. In either case the env would be useful.

This was part of a "standalone" build of gkrust inside a gecko build. I am using clang 9.0.0.

I can't see this warning on a local build, but I don't have sccache-dist set up.

sccache-dist can process errors slightly differently because the compiler is working on preprocessed source, and various warnings trigger much more easily in such cases. It's possible that this is specific to that setup.

I'm hesitant to make fixes to glsl-optimizer, as it is based on mesa's source code and we want to be able to rebase it as easily as possible.

Does upstream take patches? It looked like glsl-optimizer wasn't being developed anymore.

My build script sets .warnings(false) on the cc::Build object, which I hoped would be enough to silence warnings. Perhaps this is a bug in cc?

For better or worse, we propagate gecko's warning flags into the *FLAGS variables that cc sees, so I don't think this is a cc bug.

[jamienicol]

Does upstream take patches? It looked like glsl-optimizer wasn't being developed anymore.

You are correct, upstream glsl-optimizer isn't being developed any more. It was itself a fork of (a very old version of) mesa, and our fork has been rebased on upstream mesa. This fixed a number of issues but was rather painful, so it would be good to avoid future pain as much as possible. That said, if it is indeed our best option so be it. 😄

For better or worse, we propagate gecko's warning flags into the *FLAGS variables that cc sees, so I don't think this is a cc bug.

glslopt-rs' build.rs acually tries to ignore these variables. The issue is that gecko's build system exports CFLAGS_$HOST and CFLAGS_$TARGET and, for non-cross-compiled builds where $HOST == $TARGET, the latter takes precedence. We want to build this as a host library, but the target cflags caused issues by making us include gecko headers, etc. Certainly not an ideal situation, and I wonder if a flag is making it through somehow.

I think cc should log which variable name it chooses and the value of that variable when building. Could you post that portion of the build log?

[jamienicol]

Hmm, now that I'm actually reading cc's source code, it doesn't look like Build.warnings(false) actually suppresses warnings, it just means we don't enable non-default warnings: https://github.com/alexcrichton/cc-rs/blob/master/src/lib.rs#L1285

So maybe the solution is to set C(XX)FLAGS in build.rs to explicitly disable warnings.