[clang] d374b65 - Drop qualifiers from return types in C (DR423)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu May 19 10:07:07 PDT 2022


Author: Aaron Ballman
Date: 2022-05-19T13:06:50-04:00
New Revision: d374b65f2da1bdd3d9a7e9ac8ed4ad5467c882f9

URL: https://github.com/llvm/llvm-project/commit/d374b65f2da1bdd3d9a7e9ac8ed4ad5467c882f9
DIFF: https://github.com/llvm/llvm-project/commit/d374b65f2da1bdd3d9a7e9ac8ed4ad5467c882f9.diff

LOG: 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

Added: 
    clang/test/Sema/wg14-dr423.c

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: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4fb4c662d71b1..69b5dc1dd1fd3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
 -------------------

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index da44a0405b05c..384a8ce3314d4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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">;

diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 05a9b56f7d8f6..314110c926828 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/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

diff  --git a/clang/test/CodeGen/xcore-stringtype.c b/clang/test/CodeGen/xcore-stringtype.c
index 4b9429caa1df3..365a36dc60ccd 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{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);

diff  --git a/clang/test/Sema/block-call.c b/clang/test/Sema/block-call.c
index 0b96c3bee5e33..81071121b6adf 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;       // 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;

diff  --git a/clang/test/Sema/c89.c b/clang/test/Sema/c89.c
index 0bbd14eca6b41..67b05eab24e43 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) {} /* 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}} */

diff  --git a/clang/test/Sema/function.c b/clang/test/Sema/function.c
index b4ecc54559c7c..c41e3a69d9e0d 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) // expected-warning {{function cannot return qualified void type 'const void'}}
+void const Bar (void) // also okay on defn per DR 423
 {
 }

diff  --git a/clang/test/Sema/warn-missing-prototypes.c b/clang/test/Sema/warn-missing-prototypes.c
index 37176c66de4b6..debb7f8141707 100644
--- a/clang/test/Sema/warn-missing-prototypes.c
+++ b/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 at -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 at -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;
 }

diff  --git a/clang/test/Sema/wg14-dr423.c b/clang/test/Sema/wg14-dr423.c
new file mode 100644
index 0000000000000..1ce44dcffad66
--- /dev/null
+++ b/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.
+}

diff  --git a/clang/test/SemaObjC/block-omitted-return-type.m b/clang/test/SemaObjC/block-omitted-return-type.m
index f4ab4628013f0..0504b1feabcab 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 { //expected-error {{incompatible block pointer types initializing 'void (^)(void)' with an expression of type 'const void (^)(void)'}}
-    return; // expected-warning at -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;

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
index 6723e1657c1ab..3901a48245eb7 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ b/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) {

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersTest.h b/clang/unittests/ASTMatchers/ASTMatchersTest.h
index 79c6186054839..657ed46b082cc 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ b/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};


        


More information about the cfe-commits mailing list