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 atomic type? And if it is an atomic type, isn't
> > that an initialization of a temporary? In what situation does it make
> > sense to order the initialization of a temporary?
> In this case:
>
> ```
> void bad_assign_1(int i) {
> atom = i; // expected-warning {{implicit use of sequentially-consistent
> atomic may incur stronger memory barriers than necessary}}
> }
> ```
>
> We want to warn on assignment to atomics being implicitly `seq_cst`.
>
> Though you're right, initialization shouldn't warn because it isn't atomic.
> The issue is that `ATOMIC_VAR_INIT` is what needs to get used, and that's
> weird to test. I'll add a test that just assigns (which is what
> `ATOMIC_VAR_INIT` expands to for clang), and I'll need to update the code to
> detect that pattern and avoid warning in that case. I guess I have to look at
> the `Expr` and figure out if the LHS is a `Decl` or something like that.
Do we really implicitly convert the RHS of that assignment to atomic type?
That seems wrong; `_Atomic` is really a type qualifier, and the RHS should not
be converted to `_Atomic(T)` any more than it would be converted to `volatile
T` for an assignment into a `volatile` l-value.
Repository:
rC Clang
https://reviews.llvm.org/D51084
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
