gribozavr added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp:49
+ diag(OperatorLoc, "POSIX functions (except posix_openpt) never return
negative values")
+ << FixItHint::CreateReplacement(OperatorLoc, Twine(">").str());
+ return;
----------------
jcai19 wrote:
> gribozavr wrote:
> > Please add tests for fix-its.
> The fix-it is triggered when cases like posix_*(...) < 0 are detected (except
> posix_openpt) , which are currently covered in the unit test. The test
> runtime will make sure fix-its are applied.
Could you add CHECK-FIXES lines to validate the fixit directly?
================
Comment at: clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp:96
+ if (posix_fadvise(0, 0, 0, 0) < 0) {}
+ if (posix_fadvise(0, 0, 0, 0) >= 0) {} else {}
+ if (posix_fadvise(0, 0, 0, 0) == -1) {}
----------------
jcai19 wrote:
> gribozavr wrote:
> > Shouldn't there be a warning in the else branch?
> This check only matches calls to the POSIX functions in the global scope, not
> member functions or functions defined within namespaces. Although in the
> global scope, this particular case will still pass as there won't be a ``<``
> binary operator for the else branch so no matching will happen.
Sorry, yes, that's what I meant -- if we warn on `< 0`, then we should warn on
the else branch of `>=`. I just commented on the wrong test -- sorry about that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63623/new/
https://reviews.llvm.org/D63623
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
