[clang] c745f2c - Revert "Drop qualifiers from return types in C (DR423)"

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 2 05:28:57 PDT 2022


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 at -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 at -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 at -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};


        


More information about the cfe-commits mailing list