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

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Thanks for the feedback!

> You noted, that the C++ warning would catch this case, but does not get triggered in C. Would it be wort to generalize the concern and have a warning that catch the comparison, regardless of the macro?

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.

> Do other major libraries define a similar macro but name it differently? If so, including there names would be worthwhile.

Dunno. A quick search for alternatives on MacOS said "no; #define it yourself," and I know so little about Windows that I have no clue if a `TEMP_FAILURE_RETRY`-like macro would even be reasonable there. :)

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


https://reviews.llvm.org/D45059