[PATCH] D51084: Implement -Watomic-implicit-seq-cst

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 23 10:09:43 PDT 2018


rjmccall added inline comments.


================
Comment at: lib/Sema/SemaChecking.cpp:4924
+        << Callee->getSourceRange();
+  }
+
----------------
Why is the diagnostic at the end location?  And why are you separately checking whether it's ignored at the begin location?


================
Comment at: lib/Sema/SemaChecking.cpp:10003
+static void DiagnoseImplicitAtomicSeqCst(Sema &S, Expr *E) {
+  if (S.Diags.isIgnored(diag::warn_atomic_implicit_seq_cst, E->getBeginLoc()))
+    return;
----------------
`isIgnored` is not actually a cheap operation; you should *definitely* be checking for atomic types first.  And usually we just fire off the diagnostic without checking `isIgnored` because the setup cost of a diagnostic is not assumed to be that high.


Repository:
  rC Clang

https://reviews.llvm.org/D51084





More information about the cfe-commits mailing list