core, log: fix tautology compare error with toolchain llvm#6744
core, log: fix tautology compare error with toolchain llvm#6744kaspar030 merged 1 commit intoRIOT-OS:masterfrom
Conversation
jnohlgard
left a comment
There was a problem hiding this comment.
Change looks good, did not yet test it.
core/include/log.h
Outdated
| * @brief Log message if level <= LOG_LEVEL | ||
| */ | ||
| #ifdef __clang__ | ||
| #define LOG(level, ...) \ |
There was a problem hiding this comment.
Please add a short comment here why we need to ignore this warning, and in what version of Clang you are seeing this problem. Just for future reference when someone finds this code and wonder why the special handling of Clang.
|
I don't know what happened in between, but currently I cannot reproduce the error. Not with clang 3.8 or 3.9 on Ubuntu Linux nor on macOS 😕 Strange ... |
c2a66ee to
f2e90c1
Compare
|
ping! Jenkins is happy, I'll trigger murdock but it should find nothing, because it doesn't check llvm toolchain 😞 |
core/include/log.h
Outdated
| #define LOG(level, ...) \ | ||
| _Pragma("clang diagnostic push") \ | ||
| _Pragma("clang diagnostic ignored \"-Wtautological-compare\"") \ | ||
| if (level <= LOG_LEVEL) log_write(level, __VA_ARGS__) \ |
There was a problem hiding this comment.
There's an error in the original line, but maybe you could fix it while you're at it: level should be parenthesized at all instances, or an invocation like LOG(quite_serious ? LOG_WARN : LOG_INFO, "Something's bad"); would fail.
Also I would surround the whole block in do { } while (0) even if it should not™ be needed.
1c4e199 to
aa88ea0
Compare
|
@Kijewski addressed your comments/suggestions, thanks! |
aa88ea0 to
b74d0db
Compare
|
please squash! |
b74d0db to
1b69f28
Compare
|
@kaspar030 or @gebart ready for an ACK and merge this one? |
|
&go |
replaces #6612, see comments there