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

This revision was automatically updated to reflect the committed changes. Closed by commit rC341860: Implement -Watomic-implicit-seq-cst (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D51084?vs=164235&id=164746#toc Repository: rC Clang https://reviews.ll

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

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/Sema/SemaChecking.cpp:10974 + if (E->IgnoreParenImpCasts()->getType()->isAtomicType()) +return; CheckImplicitConversion(S, E->IgnoreParenImp

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

jfb updated this revision to Diff 164235. jfb marked 4 inline comments as done. jfb added a comment. - Address comments. Repository: rC Clang https://reviews.llvm.org/D51084 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/atomic-implicit-seq_cst.c

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

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10974 + if (E->IgnoreParenImpCasts()->getType()->isAtomicType()) +return; CheckImplicitConversion(S, E->IgnoreParenImpCasts(), S.Context.BoolTy, CC); rjmccall wrote: > Can you explain this o

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

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

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

jfb updated this revision to Diff 163577. jfb added a comment. - Don't diagnose initialization, only assignment. Repository: rC Clang https://reviews.llvm.org/D51084 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/atomic-implicit-seq_cst.c test/S

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

jfb added a comment. In https://reviews.llvm.org/D51084#1216913, @rjmccall wrote: > It says the type of the assignment expression, not the type of the LHS. > > C11 [6.5.16]p2: "The type of an assignment expression is the type the left > operand would have after lvalue conversion." > > C11 [6.3.2

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

rjmccall added a comment. It says the type of the assignment expression, not the type of the LHS. C11 [6.5.16]p2: "The type of an assignment expression is the type the left operand would have after lvalue conversion." C11 [6.3.2.1]p2: "...this is called lvalue conversion. If the lvalue has qua

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

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10668 + if (Source->isAtomicType() || Target->isAtomicType()) +S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst); + rjmccall wrote: > jfb wrote: > > rjmccall wrote: > > > Why would t

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

rjmccall added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10668 + if (Source->isAtomicType() || Target->isAtomicType()) +S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst); + jfb wrote: > rjmccall wrote: > > Why would the target be an a

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

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10668 + if (Source->isAtomicType() || Target->isAtomicType()) +S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst); + rjmccall wrote: > Why would the target be an atomic type? And if

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

rjmccall added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10668 + if (Source->isAtomicType() || Target->isAtomicType()) +S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst); + Why would the target be an atomic type? And if it is an atom

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

jfb added a comment. Updated. 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

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

jfb updated this revision to Diff 162484. jfb marked 2 inline comments as done. jfb added a comment. - Address John's comments: diagnose at beginning, and don't check isIgnored manually. Repository: rC Clang https://reviews.llvm.org/D51084 Files: include/clang/Basic/DiagnosticSemaKinds.td

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

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?

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

jfb created this revision. jfb added a reviewer: rjmccall. Herald added subscribers: cfe-commits, dexonsmith. _Atomic and __sync_* operations are implicitly sequentially-consistent. Some codebases want to force explicit usage of memory order instead. This warning allows them to know where implicit