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 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.
I don't make the rules man! I just enforce (and try to warn) on them!

C17:

> 6.5.16.1 Simple assignment
> **Constraints**
> One of the following shall hold: 114)
> — the left operand has atomic, qualified, or unqualified arithmetic type, and 
> the right has arithmetic type;
> — the left operand has an atomic, qualified, or unqualified version of a 
> structure or union type compatible with the type of the right;
> — the left operand has atomic, qualified, or unqualified pointer type, and 
> (considering the type the left operand would have after lvalue conversion) 
> both operands are pointers to qualified or unqualified versions of compatible 
> types, and the type pointed to by the left has all the qualifiers of the type 
> pointed to by the right;
> — the left operand has atomic, qualified, or unqualified pointer type, and 
> (considering the type the left operand would have after lvalue conversion) 
> one operand is a pointer to an object type, and the other is a pointer to a 
> qualified or unqualified version of void, and the type pointed to by the left 
> has all the qualifiers of the type pointed to by the right;
> — the left operand is an atomic, qualified, or unqualified pointer, and the 
> right is a null pointer constant; or
> — the left operand has type atomic, qualified, or unqualified _Bool, and the 
> right is a pointer.
>
> **Semantics**
> In simple assignment (=), the value of the right operand is converted to the 
> type of the assignment expression and replaces the value stored in the object 
> designated by the left operand.
>
> 114)The asymmetric appearance of these constraints with respect to type 
> qualifiers is due to the conversion (specified in 6.3.2.1) that changes 
> lvalues to "the value of the expression" and thus removes any type qualifiers 
> that were applied to the 
> typecategoryoftheexpression(forexample,itremovesconstbutnotvolatilefromthetypeint
>  volatile * const).


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

Reply via email to