Skip to content

[C2y] Correctly handle incomplete types in generic selections #141596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 27, 2025
Merged

[C2y] Correctly handle incomplete types in generic selections #141596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

D, Types[i]->getTypeLoc().getBeginLoc()) >=
DiagnosticsEngine::Error)
TypeErrorFound = true;
}

// C11 6.5.1.1p2 "No two generic associations in the same generic
Expand Down
29 changes: 25 additions & 4 deletions clang/test/C/C2y/n3409.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
)
);
2 changes: 1 addition & 1 deletion clang/test/SemaCXX/generic-selection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down