This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] support: Add TEST_COMPARE macro
On 11/24/2017 07:31 PM, Paul Eggert wrote:
Florian Weimer wrote:
I expect that GCC will eventually warn about such tautological
comparisons. The current approach avoids such warnings.
I've found such warnings to be more trouble than they're worth, in cases
like these. (Why would we want GCC to warn that it's doing an
optimization? We *like* optimizations! :-) And efforts to suppress such
warnings, such as using "x > 0" instead of "x < 0", typically cause the
compiler to generate worse code.
If performance is not important here then I suppose it's OK. However,
it's a bad habit and I don't want the habit to leak into code where
performance matters.
It's for writing tests. Performance does not matter. And even for the
tests, the additional which cannot be optimized away is on the failure path.
I don't know what the context for this new macro is, and like Andreas
I'm a bit puzzled as to its intended use.
The purpose is to check if two values are the same, and print them
(and record a test failure) if they are not.
Why would we want to compare (say) an uintptr_t value with a pid_t
value? That sounds like a typo, and I'd rather see a compile-time
diagnostic for the typo. It should be easy to arrange for such a
diagnostic, and then we won't have to worry about run-time sign checking
or pacifying GCC about comparisons.
The POSIX interfaces are not strongly typed. Types like pid_t should
have been structs, but compilers and ABIs simply weren't ready for that.
More importantly for us right now is that comparisons between actual and
expected values often have differing types. Concrete types vary between
architectures, and not just their sizes (e.g., 32-bit architectures use
longs where 64-bit architectures use int). Considering that the macro
tries very hard to avoid bit-altering conversions, the worst you can get
is a spurious test failure, so I don't see why this is a problem.
(I could perhaps add an assert that the argument types are not floating
point types. But we use floating point values so rarely that this
didn't occur to me earlier.)
Thanks,
Florian