[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 30 13:59:37 PDT 2018


JonasToth added a comment.

> I'm not quite sure how we would go about that with confidence. The code we'd need to catch looks something like:
> 
> typeof(foo() == y) x;
>  /* snip */
>  bar(x == -1); // warning: comparison is pointless
> 
> If we could tell that x's type was inferred from a comparison op, we could warn here. However, if the user used x as anything but a C++ bool in /* snip */, the warning would be incorrect. We could maybe do some sort of range analysis by walking the AST, but that sounds like more like a static analyzer thing.
> 
> ...And none of that would catch glibc's TEMP_FAILURE_RETRY macro, unless we wanted to apply said analysis to all integral variables.

Yes that sounds reasonable. As i am no good C programer i have little insight in these issues. Please correct me if i am wrong!

C89 has no `bool` type and no `stdbool.h` but it has been added to later standards? That means the generalization //could// theoretically be done for later C standards, because we could check if the `bool` typedef had been used? If yes, would the check benefit?

I do not want to overthink the whole issue, because your check addresses a valid usecase and has general appliance. But if we can do more/better, we should do it :)

> If you have anything in mind, I'm happy to add it.

No clue :D I think @aaron.ballman knows C good and might be a better reviewer for this code in general :)


https://reviews.llvm.org/D45059





More information about the cfe-commits mailing list