Drop qualifiers from return types in C (DR423)

WG14 DR423 (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2148.htm#dr_423),
resolved during the C11 time frame, changed the way qualifiers are
handled on function return types and in cast expressions after it was
noted that these types are now directly observable via generic
selection expressions. In C, the function declarator is adjusted to
ignore all qualifiers (including _Atomic qualifiers).

Clang already handles the cast expression case correctly (by performing
the lvalue conversion, which drops the qualifiers as well), but with
these changes it will now also handle function declarations
appropriately.

Fixes #39595

Differential Revision: https://reviews.llvm.org/D125919

Author: AaronBallman

clang/docs/ReleaseNotes.rst

@@ -340,6 +340,11 @@ 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
 -------------------

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6135,9 +6135,6 @@ 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">;

clang/lib/Sema/SemaType.cpp

@@ -5181,15 +5181,11 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       if ((T.getCVRQualifiers() || T->isAtomicType()) &&
           !(S.getLangOpts().CPlusPlus &&
             (T->isDependentType() || T->isRecordType()))) {
-        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);
+        // 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();
 
         // C++2a [dcl.fct]p12:
         //   A volatile-qualified return type is deprecated

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{crv:p(cv:si)}(p(cv:si))"}
-// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @eQ, !"f{crv:p(cv:si)}(p(cv:si))"}
+// 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))"}
 extern void eI();
 void gI() {eI();};
 extern void eV(void);

clang/test/Sema/block-call.c

@@ -22,7 +22,7 @@ int main(void) {
 
   int * const (^IPCC1) () = IPCC;
 
-  int * (^IPCC2) () = IPCC;       // expected-error {{incompatible block pointer types initializing 'int *(^)()' with an expression of type 'int *const (^)()'}}
+  int * (^IPCC2) () = IPCC;       // OK per WG14 DR 423 because the 'const' was dropped from the declarator.
 
   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 *const (^)()' from 'int'}}
+  IPCC1 = 1; // expected-error {{invalid block pointer conversion assigning to 'int *(^)()' 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;

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) {} /* expected-error {{'main' must return 'int'}} */
+const int main(void) {} /* OK per DR 423 */
 
 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}} */

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) // expected-warning {{function cannot return qualified void type 'const void'}}
+void const Bar (void) // also okay on defn per DR 423
 {
 }

clang/test/Sema/warn-missing-prototypes.c

@@ -58,9 +58,14 @@ 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]]:1-[[@LINE-2]]:1}:"static "
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static "
   struct MyStruct ret;
   return ret;
 }
@@ -70,7 +75,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]]:1-[[@LINE-2]]:1}:"static "
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:8-[[@LINE-2]]:8}:"static "
   struct MyStruct ret;
   return ret;
 }

clang/test/Sema/wg14-dr423.c

@@ -0,0 +1,31 @@
+// 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.
+}

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 { //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 (^simpleBlock5)(void) = ^ const void { // OK after DR 423.
+    return;
   };
   void (^simpleBlock6)(void) = ^ const (void) { //expected-warning {{'const' qualifier on omitted return type '<dependent type>' has no effect}}
     return;

clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

@@ -2095,8 +2095,18 @@ TEST_P(ASTMatchersTest, QualifiedTypeLocTest_BindsToConstIntVarDecl) {
 }
 
 TEST_P(ASTMatchersTest, QualifiedTypeLocTest_BindsToConstIntFunctionDecl) {
-  EXPECT_TRUE(matches("const int f() { return 5; }",
-                      qualifiedTypeLoc(loc(asString("const int")))));
+  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()));
 }
 
 TEST_P(ASTMatchersTest, QualifiedTypeLocTest_DoesNotBindToUnqualifiedVarDecl) {

clang/unittests/ASTMatchers/ASTMatchersTest.h

@@ -60,6 +60,17 @@ 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};