aaron.ballman added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/bugprone/SpuriouslyWakeUpFunctionsCheck.cpp:22
+  auto hasUniqueLock = hasDescendant(declRefExpr(hasDeclaration(
+      varDecl(hasType(asString("std::unique_lock<std::mutex>"))))));
+
----------------
These should all use `::std::whatever` to guard against pathological code that 
uses `std` inside of another namespace.


================
Comment at: 
clang-tools-extra/clang-tidy/bugprone/SpuriouslyWakeUpFunctionsCheck.cpp:80
+    Str << " or used with a condition parameter";
+  diag(MatchedWait->getExprLoc(), Str.str().str()) << waitName;
+}
----------------
This should be simplified into:
```
const auto *MatchedWait = Result.Nodes.getNodeAs<CallExpr>("wait");
StringRef WaitName = MatchedWait->getDirectCallee()->getName();
diag(MatchedWait->getExprLoc(), "'%0' should be placed inside a while statement 
%select{|or used with a conditional parameter}1") << WaitName << (WaitName != 
"cnd_wait" && WaitName != cnd_timedwait);
```


================
Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-spuriously-wake-up-functions.rst:6
+
+Finds ``cnd_wait``, ``wait``, ``wait_for``, or ``wait_until`` function calls
+when the function is not invoked from a loop that checks whether a condition
----------------
Missing `cnd_timedwait`


================
Comment at: 
clang-tools-extra/test/clang-tidy/Inputs/bugprone-spuriously-wake-up-functions/chrono.h:49
+} // namespace std
\ No newline at end of file

----------------
Please add a newline to the end of the file.


================
Comment at: 
clang-tools-extra/test/clang-tidy/Inputs/bugprone-spuriously-wake-up-functions/condition_variable.h:41
+} // namespace std
\ No newline at end of file

----------------
Please add a newline to the end of the file.


================
Comment at: 
clang-tools-extra/test/clang-tidy/Inputs/bugprone-spuriously-wake-up-functions/cstdint.h:4
+}
\ No newline at end of file

----------------
Please add a newline to the end of the file.


================
Comment at: 
clang-tools-extra/test/clang-tidy/Inputs/bugprone-spuriously-wake-up-functions/mutex.h:20
+} // namespace std
\ No newline at end of file

----------------
Please add a newline to the end of the file.


================
Comment at: 
clang-tools-extra/test/clang-tidy/Inputs/bugprone-spuriously-wake-up-functions/ratio.h:29
+} // namespace std
\ No newline at end of file

----------------
Please add a newline to the end of the file.


================
Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-spuriously-wake-up-functions.cpp:2
+// RUN: %check_clang_tidy %s bugprone-spuriously-wake-up-functions %t -- -- -I 
%S/Inputs/bugprone-spuriously-wake-up-functions/
+#include "condition_variable.h"
+#define NULL 0
----------------
Are you planning to use these declarations in multiple test files? If not, then 
we typically put all of the header code into the test file rather than added as 
explicit files.


================
Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-spuriously-wake-up-functions.cpp:99
+  }
+}
----------------
You are missing test cases that use other kinds of loops, like a `do..while` or 
`for` loop.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70876/new/

https://reviews.llvm.org/D70876



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to