[clang] [clang][Sema] Fix type of an statement expression ending with an atomic type (PR #119711)

[via cfe-commits]

Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/119711 at github.com>


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Alejandro Álvarez Ayllón (alejandro-alvarez-sonarsource)

<details>
<summary>Changes</summary>

When a statement expression's last statement is an atomic variable, GCC and Clang disagree on the type of the expression. This can be made apparent using `typeof` and forcing a diagnostic message:

```cpp
_Atomic int a = 0;
typeof(({a;})) x = "0";
```

* GCC complains about initializing `int` with `char*`
* Clang complains about initializing `_Atomic(int)` with a `char[2]`

Due to the type of the statement expression being deduced to be atomic, we end with three implicit casts inside the `StmtExpr` on the AST:

* `LValueToRValue` -> `AtomicToNonAtomic`  ->  `NonAtomicToAtomic`

In some situations, this can end on an assertion inside `IntExprEvaluator`, as reported in #<!-- -->106576.

With this patch, we now have two implicit casts, since the type of the statement expression is deduced to be non-atomic:

* `LValueToRValue` -> `AtomicToNonAtomic`

This is consistent with the C standard (6.7.2.4, p4)

>  The properties associated with atomic types are meaningful only for expressions that are lvalues.

But a statement expression is an rvalue.

`IntExprEvaluator` assumptions are now satisfied and there is no assertion error.
Additionally, the `typeof` trick mentioned above shows that the type is consistently deduced between GCC and Clang.

Fixes #<!-- -->106576

---
Full diff: https://github.com/llvm/llvm-project/pull/119711.diff


2 Files Affected:

- (modified) clang/lib/Sema/SemaExpr.cpp (+9-2) 
- (added) clang/test/Sema/gh106576.c (+16) 


``````````diff
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 20bf6f7f6f28ff..45ae97807c2032 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -15861,10 +15861,17 @@ ExprResult Sema::ActOnStmtExprResult(ExprResult ER) {
   if (Cast && Cast->getCastKind() == CK_ARCConsumeObject)
     return Cast->getSubExpr();
 
+  auto Ty = E->getType().getUnqualifiedType();
+
+  // If the type is an atomic, the statement type is the underlying type.
+  if (const AtomicType *AT = Ty->getAs<AtomicType>()) {
+    Ty = AT->getValueType().getUnqualifiedType();
+    return PerformImplicitConversion(E, Ty, AssignmentAction::Casting);
+  }
+
   // FIXME: Provide a better location for the initialization.
   return PerformCopyInitialization(
-      InitializedEntity::InitializeStmtExprResult(
-          E->getBeginLoc(), E->getType().getUnqualifiedType()),
+      InitializedEntity::InitializeStmtExprResult(E->getBeginLoc(), Ty),
       SourceLocation(), E);
 }
 
diff --git a/clang/test/Sema/gh106576.c b/clang/test/Sema/gh106576.c
new file mode 100644
index 00000000000000..792977dea14132
--- /dev/null
+++ b/clang/test/Sema/gh106576.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+typedef _Atomic char atomic_char;
+
+typedef _Atomic char atomic_char;
+
+atomic_char counter;
+
+char load_plus_one(void) {
+  return ({counter;}) + 1; // no crash
+}
+
+char type_of_stmt_expr(void) {
+  typeof(({counter;})) y = ""; // expected-error-re {{incompatible pointer to integer conversion initializing 'typeof (({{{.*}}}))' (aka 'char') with an expression of type 'char[1]'}}
+  return y;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/119711