[C11] Generic selection expressions and qualified rvalues

[AaronBallman]

It is possible to get a qualified rvalue in C. Consider:

struct { cont int i; } foo();
_Generic(foo().i, ...);

foo() returns an rvalue for the anonymous structure, the member access
expression then results in an rvalue of type const int. The lvalue to
rvalue conversion we perform on the controlling expression of the
generic selection expression then fails to strip any qualifiers because
the expression is already an rvalue.

Discussion on the WG14 reflectors suggested that the qualifiers should
still be stripped from the type of the controlling expression; the
standard should be corrected to make this more clear.

Fixes #96713

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

It is possible to get a qualified rvalue in C. Consider:

struct { cont int i; } foo();
_Generic(foo().i, ...);

foo() returns an rvalue for the anonymous structure, the member access
expression then results in an rvalue of type const int. The lvalue to
rvalue conversion we perform on the controlling expression of the
generic selection expression then fails to strip any qualifiers because
the expression is already an rvalue.

Discussion on the WG14 reflectors suggested that the qualifiers should
still be stripped from the type of the controlling expression; the
standard should be corrected to make this more clear.

Fixes #96713

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+4)
• (modified) clang/lib/Sema/SemaExpr.cpp (+17-9)
• (modified) clang/test/Sema/generic-selection.c (+10)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index da967fcdda808..10a718fe67e54 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -740,6 +740,10 @@ Bug Fixes in This Version
   negatives where the analysis failed to detect unchecked access to guarded
   data.
 
+- When passing a qualified rvalue as the controlling expression of a
+  ``_Generic`` selection expression, Clang now properly strips the qualifiers.
+  Fixes #GH96713
+
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index db44cfe1288b6..845fd44f3b39a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -1828,18 +1828,26 @@ ExprResult Sema::CreateGenericSelectionExpr(
   // Look at the canonical type of the controlling expression in case it was a
   // deduced type like __auto_type. However, when issuing diagnostics, use the
   // type the user wrote in source rather than the canonical one.
+  //
+  // Note: there is an edge case for this in C where you can get a
+  // qualified rvalue and the qualifiers are not stripped by the lvalue
+  // conversion applied above. e.g.,
+  //
+  //   struct { const int i; } foo();
+  //   _Generic(foo().i, ...);
+  //
+  // Therefore, in C, we will still ask for the non-atomic, unqualified
+  // type despite those qualifiers generally being stripped above.
+  QualType ControllingTypeToUse =
+      ControllingExpr ? ControllingExpr->getType() : ControllingType->getType();
+  ControllingTypeToUse = ControllingTypeToUse.getCanonicalType();
+  if (!getLangOpts().CPlusPlus && ControllingExpr)
+    ControllingTypeToUse = ControllingTypeToUse.getAtomicUnqualifiedType();
   for (unsigned i = 0; i < NumAssocs; ++i) {
     if (!Types[i])
       DefaultIndex = i;
-    else if (ControllingExpr &&
-             Context.typesAreCompatible(
-                 ControllingExpr->getType().getCanonicalType(),
-                 Types[i]->getType()))
-      CompatIndices.push_back(i);
-    else if (ControllingType &&
-             Context.typesAreCompatible(
-                 ControllingType->getType().getCanonicalType(),
-                 Types[i]->getType()))
+    else if (Context.typesAreCompatible(ControllingTypeToUse,
+                                        Types[i]->getType()))
       CompatIndices.push_back(i);
   }
 
diff --git a/clang/test/Sema/generic-selection.c b/clang/test/Sema/generic-selection.c
index 1f17896ca4cda..ff2fe6ec44eff 100644
--- a/clang/test/Sema/generic-selection.c
+++ b/clang/test/Sema/generic-selection.c
@@ -86,3 +86,13 @@ void GH55562(void) {
   struct S s = { 0 };
   int i = s.a;
 }
+
+struct { const int i; } GH96713_1(void);
+void GH96713_2(void) {
+  _Static_assert(           // ext-warning {{'_Static_assert' is a C11 extension}}
+    _Generic(GH96713_1().i, // ext-warning {{'_Generic' is a C11 extension}}
+      int : 1,
+      const int : 0         // expected-warning {{due to lvalue conversion of the controlling expression, association of type 'const int' will never be selected because it is qualified}}
+    ), ""
+  );
+}

[Sirraide]

Implementation LGTM.

[zygoloid]

Discussion on the WG14 reflectors suggested that the qualifiers should still be stripped from the type of the controlling expression; the standard should be corrected to make this more clear.

Does WG14 think we're right to produce a qualified rvalue in this case? Another option might be to strip qualifiers in rvalue member access, but that would have further-reaching impact.

[AaronBallman]

Discussion on the WG14 reflectors suggested that the qualifiers should still be stripped from the type of the controlling expression; the standard should be corrected to make this more clear.

Does WG14 think we're right to produce a qualified rvalue in this case? Another option might be to strip qualifiers in rvalue member access, but that would have further-reaching impact.

The sentiment on the reflector (granted, it's reflector sentiment and not asked of the whole committee in an official way) was that you do get a qualified rvalue out a member access expression and that it has utility for cases like use with typeof (consider: typeof(foo().i) x = 12;, but that we really should update the way qualifiers work so we can clean up edge cases in the language (like if the structure contained an array, there's a special case to give the returned object temporary lifetime but that might make it an lvalue or it could be an rvalue, depending on how you squint). _Generic is the only case we could find where it's possible to get an rvalue that you then perform notional lvalue conversion on, so the plan is for me to write a paper to narrowly repair _Generic by specifying the type is decayed and qualifiers are stripped rather than leaning on lvalue conversion.

[AaronBallman]

Discussion on the WG14 reflectors suggested that the qualifiers should still be stripped from the type of the controlling expression; the standard should be corrected to make this more clear.

Does WG14 think we're right to produce a qualified rvalue in this case? Another option might be to strip qualifiers in rvalue member access, but that would have further-reaching impact.

The sentiment on the reflector (granted, it's reflector sentiment and not asked of the whole committee in an official way) was that you do get a qualified rvalue out a member access expression

Now Joseph Myers has brought up the opposite:

Do you think GCC would be willing to break code
using typeof so that typeof and typeof behave the same in this
case? If so, I can check with the Clang community to see if there's an

I think it would be a reasonable change in GCC not to have qualifiers here
for either typeof or typeof, much like a series of previous fixes in
this area by Martin to avoid qualifiers on rvalues in other cases. The
one case where we deliberately make C23 typeof behave differently from
typeof (any mode) and typeof in pre-C23 GNU modes is that typeof
treats the noreturn property of a function as being part of its type.

So it's now less clear which direction to resolve this. I am thinking of putting up a question on Discourse to see whether the community would support changing __typeof__ behavior to drop qualifiers in this case despite that being a potentially breaking change. If the community says we cannot/should not, then that's good information to bring back to WG14.