[clang] Emit diagnostic for typedef+auto missed case in C++98/C23 - #210141
Conversation
|
@llvm/pr-subscribers-clang Author: Rahul (rahulana-quic) ChangesCheckTypeSpec() converted 'auto' to a storage-class specifier without This change add the check for tydef in the code handling auto. Full diff: https://github.com/llvm/llvm-project/pull/210141.diff 2 Files Affected:
diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 4d20657d5e517..b6764178e89ba 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -1244,11 +1244,19 @@ void DeclSpec::CheckTypeSpec(Sema &S, const PrintingPolicy &Policy) {
(S.getLangOpts().CPlusPlus && !S.getLangOpts().CPlusPlus11)) {
// In C23 or C++98, convert 'auto' to storage class specifier
if (TypeSpecType == TST_auto) {
- // "auto int" case: Convert 'auto' to storage class specifier
- StorageClassSpec = SCS_auto;
- StorageClassSpecLoc = TSTLoc;
- TypeSpecType = ConflictingTypeSpecifier;
- TSTLoc = ConflictingTypeSpecifierLoc;
+ // "auto int" case: Convert 'auto' to storage class specifier.
+ // But typedef + any storage-class-specifier is unconditionally invalid
+ // per [dcl.stc]p1, regardless of C++ version.
+ if (StorageClassSpec == SCS_typedef) {
+ S.Diag(TSTLoc, diag::err_invalid_decl_spec_combination)
+ << "typedef" << FixItHint::CreateRemoval(TSTLoc);
+ TypeSpecType = TST_error;
+ } else {
+ StorageClassSpec = SCS_auto;
+ StorageClassSpecLoc = TSTLoc;
+ TypeSpecType = ConflictingTypeSpecifier;
+ TSTLoc = ConflictingTypeSpecifierLoc;
+ }
// Clear the conflict tracking
ConflictingTypeSpecifier = TST_unspecified;
ConflictingTypeSpecifierLoc = SourceLocation();
@@ -1273,11 +1281,19 @@ void DeclSpec::CheckTypeSpec(Sema &S, const PrintingPolicy &Policy) {
return;
}
// int auto (without constexpr): Convert 'auto' to storage class
- // specifier. No type conflict error - auto is treated as storage class,
- // not type specifier.
- StorageClassSpec = SCS_auto;
- StorageClassSpecLoc = ConflictingTypeSpecifierLoc;
- // TypeSpecType already has the correct type (e.g., TST_int)
+ // specifier. But typedef + any storage-class-specifier is
+ // unconditionally invalid per [dcl.stc]p1.
+ if (StorageClassSpec == SCS_typedef) {
+ S.Diag(ConflictingTypeSpecifierLoc,
+ diag::err_invalid_decl_spec_combination)
+ << "typedef"
+ << FixItHint::CreateRemoval(ConflictingTypeSpecifierLoc);
+ TypeSpecType = TST_error;
+ } else {
+ StorageClassSpec = SCS_auto;
+ StorageClassSpecLoc = ConflictingTypeSpecifierLoc;
+ // TypeSpecType already has the correct type (e.g., TST_int)
+ }
// Clear the conflict tracking
ConflictingTypeSpecifier = TST_unspecified;
ConflictingTypeSpecifierLoc = SourceLocation();
@@ -1351,23 +1367,38 @@ void DeclSpec::CheckTypeSpec(Sema &S, const PrintingPolicy &Policy) {
TypeSpecType = TST_error;
TypeSpecOwned = false;
} else if (!S.getLangOpts().CPlusPlus) {
- // For C, C23, etc., convert 'auto' to storage class specifier
- // (This is already handled above for C23, but keep for other C dialects)
- // In C, C23, OpenCL, etc., convert 'auto' to storage class specifier
+ // For pre-C23 C: 'auto' is a storage-class specifier, not a type
+ // specifier. C23 is handled above; this branch covers C17 and earlier.
if (TypeSpecType == TST_auto) {
- // "auto int" case: Convert 'auto' to storage class specifier
- StorageClassSpec = SCS_auto;
- StorageClassSpecLoc = TSTLoc;
- TypeSpecType = ConflictingTypeSpecifier;
- TSTLoc = ConflictingTypeSpecifierLoc;
+ // "auto int" case: Convert 'auto' to storage class specifier.
+ // typedef + any storage-class-specifier is invalid per [dcl.stc]p1.
+ if (StorageClassSpec == SCS_typedef) {
+ S.Diag(TSTLoc, diag::err_invalid_decl_spec_combination)
+ << "typedef" << FixItHint::CreateRemoval(TSTLoc);
+ TypeSpecType = TST_error;
+ } else {
+ StorageClassSpec = SCS_auto;
+ StorageClassSpecLoc = TSTLoc;
+ TypeSpecType = ConflictingTypeSpecifier;
+ TSTLoc = ConflictingTypeSpecifierLoc;
+ }
// Clear the conflict tracking
ConflictingTypeSpecifier = TST_unspecified;
ConflictingTypeSpecifierLoc = SourceLocation();
} else if (ConflictingTypeSpecifier == TST_auto) {
- // "int auto" case: Convert 'auto' to storage class specifier
- StorageClassSpec = SCS_auto;
- StorageClassSpecLoc = ConflictingTypeSpecifierLoc;
- // TypeSpecType already has the correct type (e.g., TST_int)
+ // "int auto" case: Convert 'auto' to storage class specifier.
+ // typedef + any storage-class-specifier is invalid per [dcl.stc]p1.
+ if (StorageClassSpec == SCS_typedef) {
+ S.Diag(ConflictingTypeSpecifierLoc,
+ diag::err_invalid_decl_spec_combination)
+ << "typedef"
+ << FixItHint::CreateRemoval(ConflictingTypeSpecifierLoc);
+ TypeSpecType = TST_error;
+ } else {
+ StorageClassSpec = SCS_auto;
+ StorageClassSpecLoc = ConflictingTypeSpecifierLoc;
+ // TypeSpecType already has the correct type (e.g., TST_int)
+ }
// Clear the conflict tracking
ConflictingTypeSpecifier = TST_unspecified;
ConflictingTypeSpecifierLoc = SourceLocation();
diff --git a/clang/test/SemaCXX/auto-cxx98.cpp b/clang/test/SemaCXX/auto-cxx98.cpp
index 1e28d0635a48d..db2036d462532 100644
--- a/clang/test/SemaCXX/auto-cxx98.cpp
+++ b/clang/test/SemaCXX/auto-cxx98.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98 -Wc++11-compat
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98 -Wc++11-compat
void f() {
auto int a; // expected-warning {{'auto' storage class specifier is redundant and incompatible with C++11}}
int auto b; // expected-warning {{'auto' storage class specifier is redundant and incompatible with C++11}}
@@ -6,3 +6,11 @@ void f() {
static auto d = 0; // expected-warning {{C++11 extension}}
auto static e = 0; // expected-warning {{C++11 extension}}
}
+
+// typedef and auto storage-class-specifier cannot appear in the same
+// decl-specifier-seq ([dcl.stc] p1). This must be diagnosed in C++98 even
+// though 'auto int' (without typedef) is valid there.
+void g() {
+ typedef auto int t1; // expected-error {{cannot combine with previous 'typedef' declaration specifier}}
+ auto typedef int t2; // expected-error {{cannot combine with previous 'typedef' declaration specifier}}
+}
|
|
cc: @efriedma-quic |
|
CC @osamakader |
to268
left a comment
There was a problem hiding this comment.
I would like to see a test case for the pre-C23 behavior.
AaronBallman
left a comment
There was a problem hiding this comment.
We should also add a release note to clang/docs/ReleaseNotes.md but that can be done later because this might eventually be something we want to cherry-pick to the 23.x branch.
| // decl-specifier-seq ([dcl.stc] p1). This must be diagnosed in C++98 even | ||
| // though 'auto int' (without typedef) is valid there. | ||
| void g() { | ||
| typedef auto int t1; // expected-error {{cannot combine with previous 'typedef' declaration specifier}} |
There was a problem hiding this comment.
Aren't we solving the wrong regression?
https://godbolt.org/z/xMrh3rzfv
In 22.x we're diagnosing that you cannot combine auto and int in C++98 mode. This patch restores that behavior, but in C++98, we can combine auto and int (https://godbolt.org/z/K8GWKr7Yj), what we cannot combine is auto and typedef. But we dropped the diagnostic which said 'auto' not allowed in typedef. I think that was the diagnostic we wanted to retain, wasn't it?
There was a problem hiding this comment.
In 22.x we're diagnosing that you cannot combine auto and int in C++98 mode. This patch restores that behavior
This patch does not restore that behavior because it does not really modify the auto+int-without-typedef path in any way. I am also confused by what you said about the 22.x behavior since I see here that auto+int (no typedef) works without any issues.
But we dropped the diagnostic which said 'auto' not allowed in typedef
We just emit the diagnostic at a different/earlier point but it is still an equivalent diagnostic which correctly points out the problem:
<stdin>:1:20: error: cannot combine with previous 'typedef' declaration specifier
1 | void g() { typedef auto int t1; }
| ^~~~
1 error generated.
In the current behavior, even though we emit <source>:2:11: error: 'auto' not allowed in typedef we also emit <source>:2:16: error: cannot combine with previous 'auto' declaration specifier which is wrong because like you said, this should be allowed. After the patch, the only diagnostic is what I pointed to above and is correct.
There was a problem hiding this comment.
We just emit the diagnostic at a different/earlier point but it is still an equivalent diagnostic which correctly points out the problem:
Ah, I think this was my mistake coupled with the diagnostic wording being a bit more ambiguous unless you see the caret. The new behavior is saying cannot combine with the previous typedef specifier, it previously was saying auto specifier.
I think auto not allowed in typedef was more clear, but this is definitely equivalent when I was thinking it wasn't, sorry about that!
CheckTypeSpec() converted 'auto' to a storage-class specifier without checking whether 'typedef' was already set. [dcl.stc]p1 unconditionally forbids typedef alongside any storage-class specifier regardless of C++ version. This change adds the check for tydef in the code handling auto in the C++98/C23 paths.
ca4f116 to
f27d290
Compare
I jumped the gun a little and added code to handle some extra cases. Pre-C23 was not broken before and that behavior is left unchanged. |
AaronBallman
left a comment
There was a problem hiding this comment.
LGTM! No release note needed because I presume this should be backported to 23.x.
| // decl-specifier-seq ([dcl.stc] p1). This must be diagnosed in C++98 even | ||
| // though 'auto int' (without typedef) is valid there. | ||
| void g() { | ||
| typedef auto int t1; // expected-error {{cannot combine with previous 'typedef' declaration specifier}} |
There was a problem hiding this comment.
We just emit the diagnostic at a different/earlier point but it is still an equivalent diagnostic which correctly points out the problem:
Ah, I think this was my mistake coupled with the diagnostic wording being a bit more ambiguous unless you see the caret. The new behavior is saying cannot combine with the previous typedef specifier, it previously was saying auto specifier.
I think auto not allowed in typedef was more clear, but this is definitely equivalent when I was thinking it wasn't, sorry about that!
|
@AaronBallman thanks for reviewing. Could you please merge this for me? |
Sure can! Thank you for the poke. :-) |
|
LGTM too, thanks for the fix! |
|
/cherry-pick 34436db |
|
/cherry-pick 34436db |
|
/pull-request #211391 |
…vm#210141) CheckTypeSpec() converted 'auto' to a storage-class specifier without checking whether 'typedef' was already set. [dcl.stc]p1 unconditionally forbids typedef alongside any storage-class specifier regardless of C++ version. This change add the check for tydef in the code handling auto. (cherry picked from commit 34436db)
…vm#210141) CheckTypeSpec() converted 'auto' to a storage-class specifier without checking whether 'typedef' was already set. [dcl.stc]p1 unconditionally forbids typedef alongside any storage-class specifier regardless of C++ version. This change add the check for tydef in the code handling auto.
…/C23 (llvm#210141)" This reverts commit 34436db.
CheckTypeSpec() converted 'auto' to a storage-class specifier without
checking whether 'typedef' was already set. [dcl.stc]p1 unconditionally
forbids typedef alongside any storage-class specifier regardless of C++
version.
This change add the check for tydef in the code handling auto.