mhm, okay need to this another way. I'll see to it

for clarification, the statement:

#define LOG(level, ...) if (level <= LOG_LEVEL) log_write(level, __VA_ARGS__)

led to a tautological compare error when using toolchain LLVM. I read they also added this to gcc 6.x.y, so this will likely hit more often in the future.

[edit]
for example when LOG_LEVEL=LOG_INFO and you use LOG_INFO("Foobar") it results in if (LOG_INFO <= LOG_INFO)
[/edit]

This PR resolves the issue ...

@gebart please note that the logging defined here, has nothing to do with logging done be ENABLE_DEBUG! This logging facility is enabled when you #include "log.h" into your code and set an appropriate LOG_LEVEL, default is LOG_INFO -- while the other is done by #include "debug.h".

and set an appropriate LOG_LEVEL, default is LOG_INFO

If you don't, the log code will be kicked out by the preprocessor, which this log.h previously intentionally tried to not do.

Can this be fixed with pragmas?

@kaspar030 please elaborate, I'm unsure what you mean?

In any case with whatever LOG_LEVEL, LOG_{ERROR|WARNING|INFO|DEBUG} will be defined as (void)(0).

@kaspar030 ping!

In any case with whatever LOG_LEVEL, LOG_{ERROR|WARNING|INFO|DEBUG} will be defined as (void)(0).

Exactly that is what the log module tries to avoid. Not used debug code will end up as

if (0 < 1) {
   ...
}

... which the compile correctly marks as always true.
The idea is that the optimizer just removes the code, after it has been syntax checked.

Historically, there were proper changes to code, which did not update the corresponding DEBUG statements. Enabling DEBUG would break the compile, but our CI would not spot those as per default we compile with ENABLE_DEBUG == 0.

The next time someone wanted to actually debug, a lot of time was lost repairing broken debug statements elsewhere in the file.

That's why this PR's solution is contrary to what the module tries to achieve.

It should be possible to disable the warning (which points to expected behavior), using compiler pragmas. Sth like push/tautological-compare//pop should do.

Historically

Just realized that @gebart already explained this...

@smlng Can you try if this patch solves the problem?

https://gist.github.com/kaspar030/621c5929cb8256b5e327180b67739efd

closing, superseded by #6744