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



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

Reply via email to