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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaChecking.cpp:4924
+        << Callee->getSourceRange();
+  }
+
----------------
rjmccall wrote:
> Why is the diagnostic at the end location?  And why are you separately checking whether it's ignored at the begin location?
I guess all the other places in this function are diagnosing at the end location, so we should probably just be consistent.  Sorry for the run-around.


================
Comment at: lib/Sema/SemaChecking.cpp:10407
+  if (E->getLHS()->getType()->isAtomicType())
+    S.Diag(E->getLHS()->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
+
----------------
Why are the operator-driven diagnostics here all pointing at the LHS instead of at the operator?


================
Comment at: lib/Sema/SemaChecking.cpp:10671
+  if (Source->isAtomicType())
+    S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
+
----------------
When pointing at an arbitrary expression, it's generally consider best to point at its caret location — the result of `getLoc()` — and then highlight its source range.


================
Comment at: lib/Sema/SemaChecking.cpp:10974
+  if (E->IgnoreParenImpCasts()->getType()->isAtomicType())
+    return;
   CheckImplicitConversion(S, E->IgnoreParenImpCasts(), S.Context.BoolTy, CC);
----------------
Can you explain this one?


Repository:
  rC Clang

https://reviews.llvm.org/D51084