llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.69k stars 11.87k forks source link

Tidy: if with identical then and else branches #58596

Open MaJerle opened 2 years ago

MaJerle commented 2 years ago

This is the code:

    if (!(b->flags & MY_FLAG)) {
        if ((mstime - b->time_change) >= GET_MIN(b)) {
            b->flags |= MY_FLAG;
            lw->evt_fn(lw, b, OPR);
            b->ka.last_time = mstime;
        }
    } else {
        if ((mstime - b->ka.last_time) >= PERIOD(b)) {
            b->ka.last_time += PERIOD(b);
            ++b->ka.cnt;
            lw->evt_fn(lw, b, OPS);
        }
    }

For which clang-tidy 15.0.0-rc3 throws an error: warning: if with identical then and else branches [bugprone-branch-clone] Command is called as: clang-tidy -checks='*' file.c

How can this be same if and else branch?

llvmbot commented 2 years ago

@llvm/issue-subscribers-bug

llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-tidy

whisperity commented 2 years ago

There seem to be some macros involved here. What are their definitions? What does this code snippet expand to after pre-processing? I've tried creating a dumb reproducer but can't get the warning to pop on my end.

LebedevRI commented 2 years ago

FWIW, i've also recently seen bogus report from that check, but don't have a reproducer. In my case it seemed to have been caused by earlier errors in source code.

MaJerle commented 2 years ago

Flags are as follows (just static number):

#define MY_FLAG    ((uint16_t)0x0001)

#define GET_MIN(b)      20
#define GET_MAX(b)      30

Currently these are just static numbers, in the future plan is to replace the static number for macro with some value from structure b

b->flags is uint16_t

MaJerle commented 1 year ago

Any clue on that?

LebedevRI commented 1 year ago

Ok, here's (an unreduced) repro: pp.cpp.txt

$ clang-tidy-16 -checks=-\*,bugprone-branch-clone pp.cpp --extra-arg=-fopenmp
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "pp.cpp"
No compilation database found in /tmp or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
8 warnings generated.
/tmp/pp.cpp:74446:3: warning: if with identical then and else branches [bugprone-branch-clone]
  if (mRaw->cfa.getColorAt(0, 0) == CFAColor::RED) {
  ^
/tmp/pp.cpp:74470:5: note: else branch starts here
  } else {
    ^
Suppressed 7 warnings (7 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

Note the -fopenmp, but i suspect there is something more general going on wrong. cc @whisperity @Szelethus

juha-h commented 4 days ago

Here is similar bogus warning example:

Clang-Tidy: Conditional operator with identical true and false expressions
Declared in: re_fmt. h
Definition:
#define re_snprintf(str, size, fmt, ...)                                      \
    _re_snprintf_s((str), (size), (fmt), RE_VA_ARGS(__VA_ARGS__))
Replacement:
_re_snprintf_s((event_buf), (sizeof event_buf), ("call closed,%s,%s"),
        _Generic((0) ? (prm) : (prm),
                _Bool: sizeof(int),
                char: sizeof(int),
                unsigned char: sizeof(unsigned int),
                short: sizeof(int),
                unsigned short: sizeof(unsigned int),
                int: sizeof(int),
                unsigned int: sizeof(unsigned int),
                long: sizeof(long),
                unsigned long: sizeof(unsigned long),
                long long: sizeof(long long),
                unsigned long long: sizeof(unsigned long long),
                float: sizeof(double),
                double: sizeof(double),
                char const *: sizeof(char const *),
                char *: sizeof(char *),
                void const *: sizeof(void const *),
                void *: sizeof(void *),
                struct pl: sizeof(struct pl),
                default: sizeof(void *)),
        (prm),
        _Generic((0) ? (tone) : (tone),
                _Bool: sizeof(int),
                char: sizeof(int),
                unsigned char: sizeof(unsigned int),
                short: sizeof(int),
                unsigned short: sizeof(unsigned int),
                int: sizeof(int),
                unsigned int: sizeof(unsigned int),
                long: sizeof(long),
                unsigned long: sizeof(unsigned long),
                long long: sizeof(long long),
                unsigned long long: sizeof(unsigned long long),
                float: sizeof(double),
                double: sizeof(double),
                char const *: sizeof(char const *),
                char *: sizeof(char *),
                void const *: sizeof(void const *),
                void *: sizeof(void *),
                struct pl: sizeof(struct pl),
                default: sizeof(void *)),
        (tone), 0)