nickdesaulniers added inline comments.

================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:2219-2221
+        if (LDat1->kind() == LK_Generic || LDat2->kind() == LK_Generic) {
+          // No warning is issued in this case.
+          if (Modify && LDat1->kind() == LK_Generic) {
----------------
The double check of `LDat1->kind() == LK_Generic` is fishy to me.  Particularly 
the case where `LDat1->kind() == LK_Generic` is false but `LDat2->kind() == 
LK_Generic` is true.

This might be clearer as:
```
if (LDat2->kind() == LK_Generic)
  continue;
else if (LDat1->kind() == LK_Generic && Modify)
  *Iter1 = Fact;
else {
  ...
```
Or is there something else to this logic I'm missing?


================
Comment at: clang/test/SemaCXX/thread-safety-annotations.h:47
+// Enable thread safety attributes only with clang.
+// The attributes can be safely erased when compiling with other compilers.
+#if defined(__clang__) && (!defined(SWIG))
----------------
Is this test suite run with other compilers? If not, I think we can remove the 
case?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65184



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

Reply via email to