Author: Aaron Ballman Date: 2022-06-02T08:28:43-04:00 New Revision: c745f2ce6c03bc6d1e59cac69cc15923d4400191
URL: https://github.com/llvm/llvm-project/commit/c745f2ce6c03bc6d1e59cac69cc15923d4400191 DIFF: https://github.com/llvm/llvm-project/commit/c745f2ce6c03bc6d1e59cac69cc15923d4400191.diff LOG: Revert "Drop qualifiers from return types in C (DR423)" This reverts commit d374b65f2da1bdd3d9a7e9ac8ed4ad5467c882f9. The changes lose AST fidelity (reported in #55778), but also may be improperly dropping _Atomic qualifiers. I am rolling the changes back until I've finished discussions in WG14 about the proper resolution to DR423. Added: Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaType.cpp clang/test/CodeGen/xcore-stringtype.c clang/test/Sema/block-call.c clang/test/Sema/c89.c clang/test/Sema/function.c clang/test/Sema/warn-missing-prototypes.c clang/test/SemaObjC/block-omitted-return-type.m clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp clang/unittests/ASTMatchers/ASTMatchersTest.h Removed: clang/test/Sema/wg14-dr423.c ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b58f85680049..be21872546bb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -355,11 +355,6 @@ AIX Support C Language Changes in Clang --------------------------- -- Finished implementing support for DR423. We already correctly handled - stripping qualifiers from cast expressions, but we did not strip qualifiers - on function return types. We now properly treat the function as though it - were declarated with an unqualified, non-atomic return type. Fixes - `Issue 39595 <https://github.com/llvm/llvm-project/issues/39595>`_. C2x Feature Support ------------------- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 0f8ed0a6e8c3..469f79a86752 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6137,6 +6137,9 @@ def note_exits_block_captures_non_trivial_c_struct : Note< def note_exits_compound_literal_scope : Note< "jump exits lifetime of a compound literal that is non-trivial to destruct">; +def err_func_returning_qualified_void : ExtWarn< + "function cannot return qualified void type %0">, + InGroup<DiagGroup<"qualified-void-return-type">>; def err_func_returning_array_function : Error< "function cannot return %select{array|function}0 type %1">; def err_field_declared_as_function : Error<"field %0 declared as a function">; diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 5c4d3b9c2a29..3c1b2931efc7 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -5182,11 +5182,15 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, if ((T.getCVRQualifiers() || T->isAtomicType()) && !(S.getLangOpts().CPlusPlus && (T->isDependentType() || T->isRecordType()))) { - // WG14 DR 423 updated 6.7.6.3p4 to have the function declarator drop - // all qualifiers from the return type. - diagnoseRedundantReturnTypeQualifiers(S, T, D, chunkIndex); - if (!S.getLangOpts().CPlusPlus) - T = T.getAtomicUnqualifiedType(); + if (T->isVoidType() && !S.getLangOpts().CPlusPlus && + D.getFunctionDefinitionKind() == + FunctionDefinitionKind::Definition) { + // [6.9.1/3] qualified void return is invalid on a C + // function definition. Apparently ok on declarations and + // in C++ though (!) + S.Diag(DeclType.Loc, diag::err_func_returning_qualified_void) << T; + } else + diagnoseRedundantReturnTypeQualifiers(S, T, D, chunkIndex); // C++2a [dcl.fct]p12: // A volatile-qualified return type is deprecated diff --git a/clang/test/CodeGen/xcore-stringtype.c b/clang/test/CodeGen/xcore-stringtype.c index 365a36dc60cc..4b9429caa1df 100644 --- a/clang/test/CodeGen/xcore-stringtype.c +++ b/clang/test/CodeGen/xcore-stringtype.c @@ -42,8 +42,8 @@ double _Complex Complex; // not supported // CHECK: !{{[0-9]+}} = !{void ()* @eV, !"f{0}(0)"} // CHECK: !{{[0-9]+}} = !{void (i32, ...)* @gVA, !"f{0}(si,va)"} // CHECK: !{{[0-9]+}} = !{void (i32, ...)* @eVA, !"f{0}(si,va)"} -// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @gQ, !"f{p(cv:si)}(p(cv:si))"} -// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @eQ, !"f{p(cv:si)}(p(cv:si))"} +// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @gQ, !"f{crv:p(cv:si)}(p(cv:si))"} +// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @eQ, !"f{crv:p(cv:si)}(p(cv:si))"} extern void eI(); void gI() {eI();}; extern void eV(void); diff --git a/clang/test/Sema/block-call.c b/clang/test/Sema/block-call.c index 81071121b6ad..0b96c3bee5e3 100644 --- a/clang/test/Sema/block-call.c +++ b/clang/test/Sema/block-call.c @@ -22,7 +22,7 @@ int main(void) { int * const (^IPCC1) () = IPCC; - int * (^IPCC2) () = IPCC; // OK per WG14 DR 423 because the 'const' was dropped from the declarator. + int * (^IPCC2) () = IPCC; // expected-error {{incompatible block pointer types initializing 'int *(^)()' with an expression of type 'int *const (^)()'}} int (^IPCC3) (const int) = PFR; @@ -33,7 +33,7 @@ int main(void) { int (^IPCC6) (int, char (^CArg) (float)) = IPCC4; // expected-error {{incompatible block pointer types initializing 'int (^)(int, char (^)(float))' with an expression of type 'int (^)(int, char (^)(double))'}} IPCC2 = 0; - IPCC1 = 1; // expected-error {{invalid block pointer conversion assigning to 'int *(^)()' from 'int'}} + IPCC1 = 1; // expected-error {{invalid block pointer conversion assigning to 'int *const (^)()' from 'int'}} int (^x)() = 0; int (^y)() = 3; // expected-error {{invalid block pointer conversion initializing 'int (^)()' with an expression of type 'int'}} int a = 1; diff --git a/clang/test/Sema/c89.c b/clang/test/Sema/c89.c index 67b05eab24e4..0bbd14eca6b4 100644 --- a/clang/test/Sema/c89.c +++ b/clang/test/Sema/c89.c @@ -107,7 +107,7 @@ const array_of_pointer_to_CI mine3; void main(void) {} /* expected-error {{'main' must return 'int'}} */ -const int main(void) {} /* OK per DR 423 */ +const int main(void) {} /* expected-error {{'main' must return 'int'}} */ long long ll1 = /* expected-warning {{'long long' is an extension when C99 mode is not enabled}} */ -42LL; /* expected-warning {{'long long' is an extension when C99 mode is not enabled}} */ diff --git a/clang/test/Sema/function.c b/clang/test/Sema/function.c index c41e3a69d9e0..b4ecc54559c7 100644 --- a/clang/test/Sema/function.c +++ b/clang/test/Sema/function.c @@ -117,6 +117,6 @@ void t22(int *ptr, int (*array)[3]) { void const Bar (void); // ok on decl // PR 20146 -void const Bar (void) // also okay on defn per DR 423 +void const Bar (void) // expected-warning {{function cannot return qualified void type 'const void'}} { } diff --git a/clang/test/Sema/warn-missing-prototypes.c b/clang/test/Sema/warn-missing-prototypes.c index debb7f814170..37176c66de4b 100644 --- a/clang/test/Sema/warn-missing-prototypes.c +++ b/clang/test/Sema/warn-missing-prototypes.c @@ -58,14 +58,9 @@ const int *get_const() { // expected-warning{{no previous prototype for function struct MyStruct {}; -// FIXME: because qualifiers are ignored in the return type when forming the -// type from the declarator, we get the position incorrect for the fix-it hint. -// It suggests 'const static struct' instead of 'static const struct'. However, -// thanks to the awful rules of parsing in C, the effect is the same and the -// code is valid, if a bit funny looking. const struct MyStruct get_struct() { // expected-warning{{no previous prototype for function 'get_struct'}} // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static " + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static " struct MyStruct ret; return ret; } @@ -75,7 +70,7 @@ const struct MyStruct get_struct() { // expected-warning{{no previous prototype // Two spaces between cost and struct const struct MyStruct get_struct_2() { // expected-warning{{no previous prototype for function 'get_struct_2'}} // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:8-[[@LINE-2]]:8}:"static " + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static " struct MyStruct ret; return ret; } diff --git a/clang/test/Sema/wg14-dr423.c b/clang/test/Sema/wg14-dr423.c deleted file mode 100644 index 1ce44dcffad6..000000000000 --- a/clang/test/Sema/wg14-dr423.c +++ /dev/null @@ -1,31 +0,0 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s -// RUN: %clang_cc1 -ast-dump %s | FileCheck %s -// expected-no-diagnostics - -void GH39595(void) { - // Ensure that qualifiers on function return types are dropped as part of the - // declaration. - extern const int const_int(void); - // CHECK: FunctionDecl {{.*}} parent {{.*}} <col:3, col:34> col:20 referenced const_int 'int (void)' extern - extern _Atomic int atomic(void); - // CHECK: FunctionDecl {{.*}} parent {{.*}} <col:3, col:33> col:22 referenced atomic 'int (void)' extern - - (void)_Generic(const_int(), int : 1); - (void)_Generic(atomic(), int : 1); - - // Make sure they're dropped from function pointers as well. - _Atomic int (*fp)(void); - (void)_Generic(fp(), int : 1); -} - -void casting(void) { - // Ensure that qualifiers on cast operations are also dropped. - (void)_Generic((const int)12, int : 1); - - struct S { int i; } s; - (void)_Generic((const struct S)s, struct S : 1); - - int i; - __typeof__((const int)i) j; - j = 100; // If we didn't strip the qualifiers during the cast, this would err. -} diff --git a/clang/test/SemaObjC/block-omitted-return-type.m b/clang/test/SemaObjC/block-omitted-return-type.m index 0504b1feabca..f4ab4628013f 100644 --- a/clang/test/SemaObjC/block-omitted-return-type.m +++ b/clang/test/SemaObjC/block-omitted-return-type.m @@ -23,8 +23,8 @@ - (void)test void (^simpleBlock4)(void) = ^ const { //expected-warning {{'const' qualifier on omitted return type '<dependent type>' has no effect}} return; }; - void (^simpleBlock5)(void) = ^ const void { // OK after DR 423. - return; + void (^simpleBlock5)(void) = ^ const void { //expected-error {{incompatible block pointer types initializing 'void (^)(void)' with an expression of type 'const void (^)(void)'}} + return; // expected-warning@-1 {{function cannot return qualified void type 'const void'}} }; void (^simpleBlock6)(void) = ^ const (void) { //expected-warning {{'const' qualifier on omitted return type '<dependent type>' has no effect}} return; diff --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp index 3901a48245eb..6723e1657c1a 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp @@ -2095,18 +2095,8 @@ TEST_P(ASTMatchersTest, QualifiedTypeLocTest_BindsToConstIntVarDecl) { } TEST_P(ASTMatchersTest, QualifiedTypeLocTest_BindsToConstIntFunctionDecl) { - StringRef Code = R"( - const int f() { return 5; } - )"; - // In C++, the qualified return type is retained. - EXPECT_TRUE(matchesConditionally( - Code, qualifiedTypeLoc(loc(asString("const int"))), true, langAnyCxx())); - // In C, the qualifications on the return type are dropped, so we expect it - // to match 'int' rather than 'const int'. - EXPECT_TRUE(matchesConditionally( - Code, qualifiedTypeLoc(loc(asString("const int"))), false, langAnyC())); - EXPECT_TRUE( - matchesConditionally(Code, loc(asString("int")), true, langAnyC())); + EXPECT_TRUE(matches("const int f() { return 5; }", + qualifiedTypeLoc(loc(asString("const int"))))); } TEST_P(ASTMatchersTest, QualifiedTypeLocTest_DoesNotBindToUnqualifiedVarDecl) { diff --git a/clang/unittests/ASTMatchers/ASTMatchersTest.h b/clang/unittests/ASTMatchers/ASTMatchersTest.h index 657ed46b082c..79c618605483 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersTest.h +++ b/clang/unittests/ASTMatchers/ASTMatchersTest.h @@ -60,17 +60,6 @@ class VerifyMatch : public MatchFinder::MatchCallback { const std::unique_ptr<BoundNodesCallback> FindResultReviewer; }; -inline ArrayRef<TestLanguage> langAnyC() { - static const TestLanguage Result[] = {Lang_C89, Lang_C99}; - return ArrayRef<TestLanguage>(Result); -} - -inline ArrayRef<TestLanguage> langAnyCxx() { - static const TestLanguage Result[] = {Lang_CXX03, Lang_CXX11, Lang_CXX14, - Lang_CXX17, Lang_CXX20}; - return ArrayRef<TestLanguage>(Result); -} - inline ArrayRef<TestLanguage> langCxx11OrLater() { static const TestLanguage Result[] = {Lang_CXX11, Lang_CXX14, Lang_CXX17, Lang_CXX20}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits