Skip to content

The misc-static-assert check warns on assert(false) in some configurations. #23254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
llvmbot opened this issue Mar 12, 2015 · 15 comments
Open
Labels
bugzilla Issues migrated from bugzilla clang-tidy confirmed Verified by a second party

Comments

@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2015

Bugzilla Link 22880
Version unspecified
OS Linux
Reporter LLVM Bugzilla Contributor
CC @ehsan,@LebedevRI,@rokups,@Xazax-hun

Extended Description

I've come up with a short repro, however it seems to depend on the standard library implementation and compiler options, so I've manually partially preprocessed the input to remove external dependencies:

$ cat test.cc
// A specific implementation of <stdbool.h>.

#define bool _Bool
#define true 1
#define false 0
#define __bool_true_false_are_defined 1

// A specific implementation of <assert.h>

extern void __assert_fail(const char *__assertion, const char *__file,
unsigned int __line, const char *__function)
attribute((noreturn));

#define assert(expr)
((expr) ? (void)(0)
: __assert_fail(#expr, FILE, LINE, ((const char *)0)))

void f() {
assert(false);
}
$ ~/work/llvm-build/bin/clang-tidy -checks=-,misc- test.cc -- -std=gnu++11
1 warning generated.
test.cc:20:3: warning: found assert() that could be replaced by static_assert() [misc-static-assert]
assert(false);
^
static_assert , ""

@llvmbot
Copy link
Member Author

llvmbot commented Mar 13, 2015

The cause of the warning is not the different implementation of assert but bool's. The checker reports cases when the false / zero literal is from a macro expansion e.g. assert(sys::IsLittleEndianHost && "Unexpected host endianness").
I can add exceptions based on the name of the macro containing the false literal. My ideas so far: false, FALSE, False, zero, ZERO, Zero. Please suggest more if you can.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 13, 2015

Handling macros 'false', 'FALSE', 'False' seems reasonable. Not sure about "zero" though. Did you see them being used somewhere?

@llvmbot
Copy link
Member Author

llvmbot commented Mar 13, 2015

It was just an idea but now I think zero shouldn't be included in the exceptions since I didn't found it used as an assert condition.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 18, 2015

I'll reuse the bug instead of closing it: there's one more case where the check complains on assert(false)/assert(0):

void abort() {}
#define assert(x) if (!(x)) abort()

void f() {
#define ASSERT0 assert(0)
ASSERT0;
}

@llvmbot
Copy link
Member Author

llvmbot commented May 19, 2015

*** Bug llvm/llvm-bugzilla-archive#23355 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Member Author

llvmbot commented Jul 15, 2015

Friendly ping. Szabolcs, are you going to have time to tackle this?

@llvmbot
Copy link
Member Author

llvmbot commented Dec 28, 2016

We use the pretty common idiom of

switch (something) {
case 0:
assert(!"This shouldn't be able to happen");
break;
case 1: ...normal handling etc
}

e.g. in case()s that ought not to be reached. misc-static-assert tags these as "replaceable with static_assert", which of course they're not, being run-time dependent on the 'something'. This seems to be a manifestation of the same bug here?

@llvmbot
Copy link
Member Author

llvmbot commented Jun 7, 2017

Re: assert(!"This shouldn't be able to happen");, this should be fixed in r304657, thanks to Florian Gross.

@LebedevRI
Copy link
Member

One more problem: it complains on the code like this:

#define ASAN_REGION_IS_POISONED(addr, size) (0)
assert(!ASAN_REGION_IS_POISONED(data, size));

@rokups
Copy link

rokups commented Jun 30, 2020

Bumped into same issue. We use our own assert macro and following cases trigger warning:

#define IM_ASSERT(_COND) assert(_COND)

IM_ASSERT(0);
IM_ASSERT(0 && "Some message");

@llvmbot
Copy link
Member Author

llvmbot commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#23355

1 similar comment
@llvmbot
Copy link
Member Author

llvmbot commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#23355

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
@nick-schultz
Copy link

@rokups Have you found a workaround for your example? We have a similar usage that you have described.

@rokups
Copy link

rokups commented Dec 9, 2022

Probably not. It was a long while ago, i do not remember for sure, sorry

@PiotrZSL
Copy link
Member

Original issue looks to be fixed by 43a298c.
Issue with IM_ASSERT is not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang-tidy confirmed Verified by a second party
Projects
None yet
Development

No branches or pull requests

5 participants