Apply function attributes through array declarators

[chill]

Summary

There's inconsistency in handling array types between the distributeFunctionTypeAttrXXX functions
and the FunctionTypeUnwrapper in SemaType.cpp.

This patch lets FunctionTypeUnwrapper apply function type attributes through
array types.

[aaron.ballman]

This will make us incompatible with how GCC behaves, won't it? Assuming yes, do you know if GCC is looking to make the same change on their end?

clang/lib/Sema/SemaType.cpp

6613

No else after a return (same for the next else as well).

6623

I'd convert this last one to use cast<> instead of dyn_cast and rely on the assertion to trigger if we got something other than an incomplete array type.

[chill]

In D75109#1895969, @aaron.ballman wrote:

This will make us incompatible with how GCC behaves, won't it? Assuming yes, do you know if GCC is looking to make the same change on their end?

GCC behaviour is rather inconsistent and IMHO is more accidental than deliberate. I have created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94196

FWIW, Clang handling of attributes already differs from GCCs. https://gcc.godbolt.org/z/jUnGER

[aaron.ballman]

I'm a bit uncomfortable with the change because the attribute appertains to a function but it is being written on an array, and this just magically "moves" the declaration around to the function type. That sort of behavior is an utter mystery of GNU-style attributes, and I am not certain that we want to add more mystery here. I'm CC'ing @rsmith and @rjmccall to see if they have opinions on the behavior.

[rjmccall]

In D75109#1924700, @chill wrote:

In D75109#1895969, @aaron.ballman wrote:

This will make us incompatible with how GCC behaves, won't it? Assuming yes, do you know if GCC is looking to make the same change on their end?

GCC behaviour is rather inconsistent and IMHO is more accidental than deliberate. I have created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94196

FWIW, Clang handling of attributes already differs from GCCs. https://gcc.godbolt.org/z/jUnGER

Okay, I tweaked this to clarify the behavior as best as I can: https://gcc.godbolt.org/z/sooFZK

Basically, GCC handles the attribute when it's directly on a function declaration, a function-pointer declaration, or a typedef to a function-pointer type, and it propagates the attribute on pointer-to-function types (includes pointers to attributed typedefs), but it doesn't accept it on more complex declarators. In contrast, Clang aggressively looks for a function type to apply it to and then propagates it reliably.

We can't pretend to be perfectly compatible with the GCC model here. Given the Clang model, I think this is a good fix.

[aaron.ballman]

In D75109#1929825, @rjmccall wrote:

In D75109#1924700, @chill wrote:

In D75109#1895969, @aaron.ballman wrote:

This will make us incompatible with how GCC behaves, won't it? Assuming yes, do you know if GCC is looking to make the same change on their end?

GCC behaviour is rather inconsistent and IMHO is more accidental than deliberate. I have created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94196

FWIW, Clang handling of attributes already differs from GCCs. https://gcc.godbolt.org/z/jUnGER

Okay, I tweaked this to clarify the behavior as best as I can: https://gcc.godbolt.org/z/sooFZK

Basically, GCC handles the attribute when it's directly on a function declaration, a function-pointer declaration, or a typedef to a function-pointer type, and it propagates the attribute on pointer-to-function types (includes pointers to attributed typedefs), but it doesn't accept it on more complex declarators. In contrast, Clang aggressively looks for a function type to apply it to and then propagates it reliably.

We can't pretend to be perfectly compatible with the GCC model here. Given the Clang model, I think this is a good fix.

I took a look through our attributes and we don't have any that would prefer to appertain to the array declaration rather than the function declaration (and I'm unaware of any such attributes in the works), which was my other concern with this patch. Coupled with what @rjmccall says above, I agree that this seems reasonable.

clang/lib/Sema/SemaType.cpp

namespace {
   ///     // change fn somehow
   ///     T = unwrapped.wrap(fn);
   ///   }
   struct FunctionTypeUnwrapper {
     enum WrapKind {
       Desugar,
       Attributed,
       Parens,
+      Array,
       Pointer,
       BlockPointer,
       Reference,
       MemberPointer,
       MacroQualified,
     };
 
     QualType Original;
     const FunctionType *Fn;
     SmallVector<unsigned char /*WrapKind*/, 8> Stack;
 
     FunctionTypeUnwrapper(Sema &S, QualType T) : Original(T) {
       while (true) {
         const Type *Ty = T.getTypePtr();
         if (isa<FunctionType>(Ty)) {
           Fn = cast<FunctionType>(Ty);
           return;
         } else if (isa<ParenType>(Ty)) {
           T = cast<ParenType>(Ty)->getInnerType();
           Stack.push_back(Parens);
+        } else if (isa<ConstantArrayType>(Ty) || isa<VariableArrayType>(Ty) ||
+                   isa<IncompleteArrayType>(Ty)) {
+          T = cast<ArrayType>(Ty)->getElementType();
+          Stack.push_back(Array);
         } else if (isa<PointerType>(Ty)) {
           T = cast<PointerType>(Ty)->getPointeeType();
           Stack.push_back(Pointer);
         } else if (isa<BlockPointerType>(Ty)) {
           T = cast<BlockPointerType>(Ty)->getPointeeType();
           Stack.push_back(BlockPointer);
         } else if (isa<MemberPointerType>(Ty)) {
           T = cast<MemberPointerType>(Ty)->getPointeeType();
QualType wrap(ASTContext &C, const Type *Old, unsigned I) {
       case Parens: {
         QualType New = wrap(C, cast<ParenType>(Old)->getInnerType(), I);
         return C.getParenType(New);
       }
 
       case MacroQualified:
         return wrap(C, cast<MacroQualifiedType>(Old)->getUnderlyingType(), I);
 
+      case Array: {
+        if (const auto *CAT = dyn_cast<ConstantArrayType>(Old)) {
+          QualType New = wrap(C, CAT->getElementType(), I);
+          return C.getConstantArrayType(New, CAT->getSize(), CAT->getSizeExpr(),
+                                        CAT->getSizeModifier(),
+                                        CAT->getIndexTypeCVRQualifiers());
+        }
+
+        if (const auto *VAT = dyn_cast<VariableArrayType>(Old)) {
+          QualType New = wrap(C, VAT->getElementType(), I);
+          return C.getVariableArrayType(
+              New, VAT->getSizeExpr(), VAT->getSizeModifier(),
+              VAT->getIndexTypeCVRQualifiers(), VAT->getBracketsRange());
+        }
+
+        const auto *IAT = cast<IncompleteArrayType>(Old);
+        QualType New = wrap(C, IAT->getElementType(), I);
+        return C.getIncompleteArrayType(New, IAT->getSizeModifier(),
+                                        IAT->getIndexTypeCVRQualifiers());
+      }
+
       case Pointer: {
         QualType New = wrap(C, cast<PointerType>(Old)->getPointeeType(), I);
         return C.getPointerType(New);
       }
 
       case BlockPointer: {
         QualType New = wrap(C, cast<BlockPointerType>(Old)->getPointeeType(),I);
         return C.getBlockPointerType(New);

clang/test/CodeGen/attr-noreturn.c

+// RUN: %clang_cc1 -S -emit-llvm %s -o - | FileCheck %s
+
+typedef void (*fptrs_t[4])(void);
+fptrs_t p __attribute__((noreturn));
+
+void __attribute__((noreturn)) f() {
+  p[0]();
+}
+// CHECK: call
+// CHECK-NEXT: unreachable

clang/test/Sema/attr-noreturn.c

 }
 
 // PR2461
 __attribute__((noreturn)) void f(__attribute__((noreturn)) void (*x)(void)) {
   x();
 }
 
 typedef void (*Fun)(void) __attribute__ ((noreturn(2))); // expected-error {{'noreturn' attribute takes no arguments}}
+
+
+typedef void fn_t(void);
+
+fn_t *fp __attribute__((noreturn));
+void __attribute__((noreturn)) f6(int i) {
+  fp();
+}
+
+fn_t *fps[4] __attribute__((noreturn));
+void __attribute__((noreturn)) f7(int i) {
+  fps[i]();
+}
+
+extern fn_t *ifps[] __attribute__((noreturn));
+void __attribute__((noreturn)) f8(int i) {
+  ifps[i]();
+}
+
+void __attribute__((noreturn)) f9(int n) {
+  extern int g9(int, fn_t **);
+  fn_t *fp[n] __attribute__((noreturn));
+  int i = g9(n, fp);
+  fp[i]();
+}
+
+typedef fn_t *fptrs_t[4];
+fptrs_t ps __attribute__((noreturn));
+void __attribute__((noreturn)) f10(int i) {
+  ps[i]();
+}