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