Fix several bugs in array -> pointer decomposition.

First, we got several CVR propagation cases wrong, which Eli pointed
out in PR2039.

Second, we didn't propagate address space qualifiers correctly, leading
to incorrect lowering of code in CodeGen/address-space.c.

Third, we didn't uniformly propagate the specifier in the array to the
pointer ("int[restrict 4]" -> "int *restrict").

This adds an ASTContext::getArrayDecayedType member that handles the
non-trivial logic for this seemingly simple operation.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@49078 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/ASTContext.h b/include/clang/AST/ASTContext.h
index c6f40b7a526bc14b2317d0bd064dc130543ab37a..897cd414c240120907a9f32146e6c0a482f2e81e 100644 (file)
--- a/include/clang/AST/ASTContext.h
+++ b/include/clang/AST/ASTContext.h
@@ -287,6 +287,14 @@ public:
   //                            Type Operators
   //===--------------------------------------------------------------------===//
   
+  /// getArrayDecayedType - Return the properly qualified result of decaying the
+  /// specified array type to a pointer.  This operation is non-trivial when
+  /// handling typedefs etc.  The canonical type of "T" must be an array type,
+  /// this returns a pointer to a properly qualified element of the array.
+  ///
+  /// See C99 6.7.5.3p7 and C99 6.3.2.1p3.
+  QualType getArrayDecayedType(QualType T);
+  
   /// maxIntegerType - Returns the highest ranked integer type. Handles 3
   /// different type combos: unsigned/unsigned, signed/signed, signed/unsigned.
   static QualType maxIntegerType(QualType lhs, QualType rhs);

diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
index 902f422dacb1f922db609ff3fe051849b7f18ea9..b9b1af1ce5414a1836a60315904e14f972a2a2a2 100644 (file)
--- a/lib/AST/ASTContext.cpp
+++ b/lib/AST/ASTContext.cpp
@@ -929,6 +929,71 @@ QualType ASTContext::getPointerDiffType() const {
   return IntTy; 
 }
 
+//===----------------------------------------------------------------------===//
+//                              Type Operators
+//===----------------------------------------------------------------------===//
+
+/// getArrayDecayedType - Return the properly qualified result of decaying the
+/// specified array type to a pointer.  This operation is non-trivial when
+/// handling typedefs etc.  The canonical type of "T" must be an array type,
+/// this returns a pointer to a properly qualified element of the array.
+///
+/// See C99 6.7.5.3p7 and C99 6.3.2.1p3.
+QualType ASTContext::getArrayDecayedType(QualType Ty) {
+  // Handle the common case where typedefs are not involved directly.
+  QualType EltTy;
+  unsigned ArrayQuals = 0;
+  unsigned PointerQuals = 0;
+  if (ArrayType *AT = dyn_cast<ArrayType>(Ty)) {
+    // Since T "isa" an array type, it could not have had an address space
+    // qualifier, just CVR qualifiers.  The properly qualified element pointer
+    // gets the union of the CVR qualifiers from the element and the array, and
+    // keeps any address space qualifier on the element type if present.
+    EltTy = AT->getElementType();
+    ArrayQuals = Ty.getCVRQualifiers();
+    PointerQuals = AT->getIndexTypeQualifier();
+  } else {
+    // Otherwise, we have an ASQualType or a typedef, etc.  Make sure we don't
+    // lose qualifiers when dealing with typedefs. Example:
+    //   typedef int arr[10];
+    //   void test2() {
+    //     const arr b;
+    //     b[4] = 1;
+    //   }
+    //
+    // The decayed type of b is "const int*" even though the element type of the
+    // array is "int".
+    QualType CanTy = Ty.getCanonicalType();
+    const ArrayType *PrettyArrayType = Ty->getAsArrayType();
+    assert(PrettyArrayType && "Not an array type!");
+    
+    // Get the element type with 'getAsArrayType' so that we don't lose any
+    // typedefs in the element type of the array.
+    EltTy = PrettyArrayType->getElementType();
+
+    // If the array was address-space qualifier, make sure to ASQual the element
+    // type.  We can just grab the address space from the canonical type.
+    if (unsigned AS = CanTy.getAddressSpace())
+      EltTy = getASQualType(EltTy, AS);
+    
+    // To properly handle [multiple levels of] typedefs, typeof's etc, we take
+    // the CVR qualifiers directly from the canonical type, which is guaranteed
+    // to have the full set unioned together.
+    ArrayQuals = CanTy.getCVRQualifiers();
+    PointerQuals = PrettyArrayType->getIndexTypeQualifier();
+  }
+  
+  // Apply any CVR qualifiers from the array type.
+  EltTy = EltTy.getQualifiedType(ArrayQuals | EltTy.getCVRQualifiers());
+
+  QualType PtrTy = getPointerType(EltTy);
+
+  // int x[restrict 4] ->  int *restrict
+  PtrTy = PtrTy.getQualifiedType(PointerQuals);
+
+  return PtrTy;
+}
+
 /// getIntegerRank - Return an integer conversion rank (C99 6.3.1.1p1). This
 /// routine will assert if passed a built-in type that isn't an integer or enum.
 static int getIntegerRank(QualType t) {

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 2d0251cc14415102c0481efe67a3c012353696f8..e247e2a52c56b0b4ddc6c02901df733837cd2dc5 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1014,10 +1014,9 @@ Sema::ActOnParamDeclarator(struct DeclaratorChunk::ParamInfo &PI,
   // we need to consider storing both types (in ParmVarDecl)...
   // 
   QualType parmDeclType = QualType::getFromOpaquePtr(PI.TypeInfo);
-  if (const ArrayType *AT = parmDeclType->getAsArrayType()) {
+  if (parmDeclType->isArrayType()) {
     // int x[restrict 4] ->  int *restrict
-    parmDeclType = Context.getPointerType(AT->getElementType());
-    parmDeclType = parmDeclType.getQualifiedType(AT->getIndexTypeQualifier());
+    parmDeclType = Context.getArrayDecayedType(parmDeclType);
   } else if (parmDeclType->isFunctionType())
     parmDeclType = Context.getPointerType(parmDeclType);
   

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index f57ad2df9c7300741ad11dd86e13b035c8ecbc80..1cd23522e8f9b33965d5837b4d3bcb2db93fa098 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -932,18 +932,8 @@ void Sema::DefaultFunctionArrayConversion(Expr *&E) {
   }
   if (Ty->isFunctionType())
     ImpCastExprToType(E, Context.getPointerType(Ty));
-  else if (const ArrayType *ArrayTy = Ty->getAsArrayType()) {
-    // Make sure we don't lose qualifiers when dealing with typedefs. Example:
-    //   typedef int arr[10];
-    //   void test2() {
-    //     const arr b;
-    //     b[4] = 1;
-    //   }
-    QualType ELT = ArrayTy->getElementType();
-    // FIXME: Handle ASQualType
-    ELT = ELT.getQualifiedType(Ty.getCVRQualifiers()|ELT.getCVRQualifiers());
-    ImpCastExprToType(E, Context.getPointerType(ELT));
-  }
+  else if (Ty->isArrayType())
+    ImpCastExprToType(E, Context.getArrayDecayedType(Ty));
 }
 
 /// UsualUnaryConversions - Performs various conversions that are common to most

diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp
index ad9d138f91b88aa6090ae983f353cc1ee42e18bb..5874477de1a29a1333d42e9d44de328a37e3aee1 100644 (file)
--- a/lib/Sema/SemaType.cpp
+++ b/lib/Sema/SemaType.cpp
@@ -317,18 +317,17 @@ QualType Sema::GetTypeForDeclarator(Declarator &D, Scope *S) {
           // type in ParmVarDecl (which makes the code generator unhappy).
           //
           // FIXME: We still apparently need the conversion in 
-          // Sema::ParseParamDeclarator(). This doesn't make any sense, since
+          // Sema::ActOnParamDeclarator(). This doesn't make any sense, since
           // it should be driving off the type being created here.
           // 
           // FIXME: If a source translation tool needs to see the original type,
           // then we need to consider storing both types somewhere...
           // 
-          if (const ArrayType *AT = ArgTy->getAsArrayType()) {
-            // int x[restrict 4] ->  int *restrict
-            ArgTy = Context.getPointerType(AT->getElementType());
-            ArgTy = ArgTy.getQualifiedType(AT->getIndexTypeQualifier());
+          if (ArgTy->isArrayType()) {
+            ArgTy = Context.getArrayDecayedType(ArgTy);
           } else if (ArgTy->isFunctionType())
             ArgTy = Context.getPointerType(ArgTy);
+          
           // Look for 'void'.  void is allowed only as a single argument to a
           // function with no other parameters (C99 6.7.5.3p10).  We record
           // int(void) as a FunctionTypeProto with an empty argument list.
@@ -391,9 +390,9 @@ QualType Sema::ObjCGetTypeForMethodDefinition(DeclTy *D) {
     assert(!ArgTy.isNull() && "Couldn't parse type?");
     // Perform the default function/array conversion (C99 6.7.5.3p[7,8]).
     // This matches the conversion that is done in 
-    // Sema::ParseParamDeclarator(). 
-    if (const ArrayType *AT = ArgTy->getAsArrayType())
-      ArgTy = Context.getPointerType(AT->getElementType());
+    // Sema::ActOnParamDeclarator(). 
+    if (ArgTy->isArrayType())
+      ArgTy = Context.getArrayDecayedType(ArgTy);
     else if (ArgTy->isFunctionType())
       ArgTy = Context.getPointerType(ArgTy);
     ArgTys.push_back(ArgTy);

diff --git a/test/CodeGen/address-space.c b/test/CodeGen/address-space.c
index 53cc827a5550fed4be39c760debb5ca10816518a..a7a08b163684196869ff0648a389460888a38004 100644 (file)
--- a/test/CodeGen/address-space.c
+++ b/test/CodeGen/address-space.c
@@ -1,4 +1,4 @@
-// RUN: clang -emit-llvm < %s 2>&1 | grep 'addrspace(1)' | count 5
+// RUN: clang -emit-llvm < %s 2>&1 | grep 'addrspace(1)' | count 6
 int foo __attribute__((address_space(1)));
 int ban[10] __attribute__((address_space(1)));
 

diff --git a/test/Sema/typedef-retain.c b/test/Sema/typedef-retain.c
index 72d3b41dbd8d4b31c77860e5c1cca5cc0274bd6f..5e4c978c3a4a2dfc3445b89b6ccab41a4b2dcc56 100644 (file)
--- a/test/Sema/typedef-retain.c
+++ b/test/Sema/typedef-retain.c
@@ -12,3 +12,15 @@ void test2(float4 a, int4p result, int i) {
     result[i] = a; // expected-error {{assigning 'float4', expected 'int4'}}
 }
 
+// PR2039
+typedef int a[5];
+void z() {
+  typedef const a b;
+  b r;
+  r[0]=10;  // expected-error {{read-only variable is not assignable}}
+}
+
+int e(const a y) {
+  y[0] = 10; // expected-error {{read-only variable is not assignable}}
+}
+