[C2y] Correctly handle incomplete types in generic selections

[AaronBallman]

We were emitting a non-error diagnostic but claiming we emitted an error, which caused some obvious follow-on problems.

Fixes #141549

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

We were emitting a non-error diagnostic but claiming we emitted an error, which caused some obvious follow-on problems.

Fixes #141549

---

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

3 Files Affected:

• (modified) clang/lib/Sema/SemaExpr.cpp (+5-3)
• (modified) clang/test/C/C2y/n3409.c (+25-4)
• (modified) clang/test/SemaCXX/generic-selection.cpp (+1-1)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 11dddac536d32..5aa33939a7817 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -1874,9 +1874,11 @@ ExprResult Sema::CreateGenericSelectionExpr(
 
         if (D != 0) {
           Diag(Types[i]->getTypeLoc().getBeginLoc(), D)
-            << Types[i]->getTypeLoc().getSourceRange()
-            << Types[i]->getType();
-          TypeErrorFound = true;
+              << Types[i]->getTypeLoc().getSourceRange() << Types[i]->getType();
+          if (getDiagnostics().getDiagnosticLevel(
+                  D, Types[i]->getTypeLoc().getBeginLoc()) >=
+              DiagnosticsEngine::Error)
+            TypeErrorFound = true;
         }
 
         // C11 6.5.1.1p2 "No two generic associations in the same generic
diff --git a/clang/test/C/C2y/n3409.c b/clang/test/C/C2y/n3409.c
index 01be716132b11..a0b8e2f28ee40 100644
--- a/clang/test/C/C2y/n3409.c
+++ b/clang/test/C/C2y/n3409.c
@@ -1,7 +1,6 @@
-// RUN: %clang_cc1 -verify -std=c2y -pedantic %s
-// RUN: %clang_cc1 -verify=pre-c2y -std=c2y -Wpre-c2y-compat %s
-// RUN: %clang_cc1 -verify=ext -std=c23 -pedantic %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -verify -std=c2y -pedantic -Wno-unused %s
+// RUN: %clang_cc1 -verify=expected,pre-c2y -std=c2y -Wpre-c2y-compat -Wno-unused %s
+// RUN: %clang_cc1 -verify=expected,ext -std=c23 -pedantic -Wno-unused %s
 
 /* WG14 N3409: Clang 21
  * Slay Some Earthly Demons X
@@ -34,3 +33,25 @@ void foo() {
   // C23 and earlier.
   return x; // ext-warning {{void function 'foo' should not return void expression}}
 }
+
+
+// Ensure we behave correctly with incomplete types. See GH141549.
+static_assert(
+  _Generic(
+    void,    /* ext-warning {{passing a type argument as the first operand to '_Generic' is a C2y extension}}
+                pre-c2y-warning {{passing a type argument as the first operand to '_Generic' is incompatible with C standards before C2y}}
+              */
+    void : 1,
+    default : 0
+  )
+);
+
+static_assert(
+  _Generic( // expected-error {{static assertion failed}}
+    12,
+    void : 1, /* ext-warning {{incomplete type 'void' in a '_Generic' association is a C2y extension}}
+                 pre-c2y-warning {{use of incomplete type 'void' in a '_Generic' association is incompatible with C standards before C2y}}
+               */
+    default : 0
+  )
+);
diff --git a/clang/test/SemaCXX/generic-selection.cpp b/clang/test/SemaCXX/generic-selection.cpp
index aa4a4c435adec..ed9d2b7d44be3 100644
--- a/clang/test/SemaCXX/generic-selection.cpp
+++ b/clang/test/SemaCXX/generic-selection.cpp
@@ -81,7 +81,7 @@ void func(struct S s) {
   // is an elaborated type specifier followed by the association's value and
   // it should work the same as in C.
   (void)_Generic(s, struct S : 1);
-  (void)_Generic(s, struct T : 1);
+  (void)_Generic(s, struct T : 1); // expected-error {{controlling expression type 'struct S' not compatible with any generic association type}}
 
   // The rest of these cases test that we still produce a reasonable diagnostic
   // when referencing an unknown type or trying to define a type in other ways.

[erichkeane]

As a nit... this sort feels like the thing we should be asking the diagnostic builder, or at least the Diagnostics Engine.

Also, kinda showing the 'downside' to the save the diagnostic id, then print it later thing here.

[AaronBallman]

Yeah, I was on the fence between setting a flag and checking the diagnostic level and went this route. But this is asking the diagnostics engine!

[erichkeane]

Well, we're asking the De the 'level' not really 'did you/would you emit an error for this'. Leading me to believe we perhaps need some sort of DiagnosticsEngine::isErrorDiagnostic or something, but that is 1/2 baked here. BUT something we should find ourselves thinking about if this happens more often.

[AaronBallman]

I think this already works the way you expect. You're basically looking for something like DiagnosticsEngine::isIgnored() but checking for errors rather than ignored, yes? If so, that function is implemented in terms of calling getDiagnosticSeverity(), but getDiagnosticLevel() is also implemented in terms of that as well. Both look at the actual state of the diagnostic.

[erichkeane]

Right, I think I'm being unclear. I'm not criticizing the functionality here, just the interface in which we're using it. IF the action we care about is always/often "is this an error", then that is the question we should allow ourselves to ask.

So something like:
getDiagnostics().willEmitError(SemaDiagnosticsBuilder&) (or, (S.Diag(...)<<...).isErrorDiag(getDiagnostics()).

Again, sorta noodling on a 'if we do this more often, we should come up with a more natural interface'.

[AaronBallman]

Ah, I guess I don't see >= Error as being an unnatural interface. :-D