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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 31 23:15:20 PDT 2018

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?

  rC Clang


More information about the cfe-commits mailing list