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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits