[cfe-commits] r41592 - in /cfe/trunk: AST/ASTContext.cpp AST/Type.cpp CodeGen/CodeGenTypes.cpp Sema/SemaDecl.cpp include/clang/AST/ASTContext.h include/clang/AST/Type.h

Steve Naroff snaroff at apple.com
Wed Aug 29 18:06:46 PDT 2007


Author: snaroff
Date: Wed Aug 29 20:06:46 2007
New Revision: 41592

URL: http://llvm.org/viewvc/llvm-project?rev=41592&view=rev
Log:

Fix the following redefinition errors submitted by Keith Bauer...

[dylan:~/llvm/tools/clang] admin% cat tentative_decls.c 
// incorrectly generates redefinition error
extern int array[3];
int array[3];

// incorrectly generates a redefinition error
extern void nup(int a[3]);
void nup(int a[3]) {}

It turns out that this exposed a fairly major flaw in the type system,
array types were never getting uniqued! This is because all array types
contained an expression, which aren't unique.

To solve this, we now have 2 array types, ConstantArrayType and
VariableArrayType. ConstantArrayType's are unique, VAT's aren't.

This is a fairly extensive set of fundamental changes. Fortunately,
all the tests pass. Nevertheless, there may be some collateral damage:-)
If so, let me know!

Modified:
    cfe/trunk/AST/ASTContext.cpp
    cfe/trunk/AST/Type.cpp
    cfe/trunk/CodeGen/CodeGenTypes.cpp
    cfe/trunk/Sema/SemaDecl.cpp
    cfe/trunk/include/clang/AST/ASTContext.h
    cfe/trunk/include/clang/AST/Type.h

Modified: cfe/trunk/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/AST/ASTContext.cpp?rev=41592&r1=41591&r2=41592&view=diff

==============================================================================
--- cfe/trunk/AST/ASTContext.cpp (original)
+++ cfe/trunk/AST/ASTContext.cpp Wed Aug 29 20:06:46 2007
@@ -162,16 +162,14 @@
   case Type::FunctionProto:
   default:
     assert(0 && "Incomplete types have no size!");
-  case Type::Array: {
-    std::pair<uint64_t, unsigned> EltInfo = 
-      getTypeInfo(cast<ArrayType>(T)->getElementType(), L);
-    
-    // Get the size of the array.
-    llvm::APSInt Sz(32);
-    if (!cast<ArrayType>(T)->getSizeExpr()->isIntegerConstantExpr(Sz, *this))
-      assert(0 && "VLAs not implemented yet!");
+  case Type::VariableArray:
+    assert(0 && "VLAs not implemented yet!");
+  case Type::ConstantArray: {
+    ConstantArrayType *CAT = cast<ConstantArrayType>(T);
     
-    Size = EltInfo.first*Sz.getZExtValue();
+    std::pair<uint64_t, unsigned> EltInfo = 
+      getTypeInfo(CAT->getElementType(), L);
+    Size = EltInfo.first*CAT->getSize().getZExtValue();
     Align = EltInfo.second;
     break;
   }    
@@ -403,37 +401,55 @@
   return QualType(New, 0);
 }
 
-/// getArrayType - Return the unique reference to the type for an array of the
-/// specified element type.
-QualType ASTContext::getArrayType(QualType EltTy,ArrayType::ArraySizeModifier ASM,
-                                  unsigned EltTypeQuals, Expr *NumElts) {
-  // Unique array types, to guarantee there is only one array of a particular
-  // structure.
+/// getConstantArrayType - Return the unique reference to the type for an 
+/// array of the specified element type.
+QualType ASTContext::getConstantArrayType(QualType EltTy, 
+                                          const llvm::APInt &ArySize) {
   llvm::FoldingSetNodeID ID;
-  ArrayType::Profile(ID, ASM, EltTypeQuals, EltTy, NumElts);
+  ConstantArrayType::Profile(ID, EltTy, ArySize);
       
   void *InsertPos = 0;
-  if (ArrayType *ATP = ArrayTypes.FindNodeOrInsertPos(ID, InsertPos))
+  if (ConstantArrayType *ATP = ArrayTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(ATP, 0);
   
   // If the element type isn't canonical, this won't be a canonical type either,
   // so fill in the canonical type field.
   QualType Canonical;
   if (!EltTy->isCanonical()) {
-    Canonical = getArrayType(EltTy.getCanonicalType(), ASM, EltTypeQuals,
-                             NumElts);
+    Canonical = getConstantArrayType(EltTy.getCanonicalType(), ArySize);
     
     // Get the new insert position for the node we care about.
-    ArrayType *NewIP = ArrayTypes.FindNodeOrInsertPos(ID, InsertPos);
+    ConstantArrayType *NewIP = ArrayTypes.FindNodeOrInsertPos(ID, InsertPos);
     assert(NewIP == 0 && "Shouldn't be in the map!");
   }
   
-  ArrayType *New = new ArrayType(EltTy, ASM, EltTypeQuals, Canonical, NumElts);
+  ConstantArrayType *New = new ConstantArrayType(EltTy, Canonical, ArySize);
   ArrayTypes.InsertNode(New, InsertPos);
   Types.push_back(New);
   return QualType(New, 0);
 }
 
+/// getArrayType - If NumElts is a constant expression, we return a unique
+/// reference to an AST node of type ConstantArrayType. If NumElts is not
+/// a constant expression, we return an instance of VaribleLengthArrayType.
+QualType ASTContext::getArrayType(QualType EltTy,
+                                  ArrayType::ArraySizeModifier ASM,
+                                  unsigned EltTypeQuals, Expr *NumElts) {
+  llvm::APSInt ArySize(32);
+  // If no expression was provided, we consider it a VLA.
+  if (!NumElts || !NumElts->isIntegerConstantExpr(ArySize, *this)) {
+    // Since we don't unique expressions, it isn't possible to unique VLA's.
+    ArrayType *New = new VariableArrayType(EltTy, ASM, EltTypeQuals, 
+                                           QualType(), NumElts);
+    Types.push_back(New);
+    return QualType(New, 0);
+  }
+  // Unique constant array types, to guarantee there is only one array of a
+  // particular structure.
+  // FIXME: should we warn if ASM != ArrayType::Normal or EltTypeQuals != 0?
+  return getConstantArrayType(EltTy, ArySize);
+}
+
 /// getVectorType - Return the unique reference to a vector type of
 /// the specified element type and size. VectorType must be a built-in type.
 QualType ASTContext::getVectorType(QualType vecType, unsigned NumElts) {

Modified: cfe/trunk/AST/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/AST/Type.cpp?rev=41592&r1=41591&r2=41592&view=diff

==============================================================================
--- cfe/trunk/AST/Type.cpp (original)
+++ cfe/trunk/AST/Type.cpp Wed Aug 29 20:06:46 2007
@@ -43,7 +43,8 @@
 bool Type::isDerivedType() const {
   switch (CanonicalType->getTypeClass()) {
   case Pointer:
-  case Array:
+  case VariableArray:
+  case ConstantArray:
   case FunctionProto:
   case FunctionNoProto:
   case Reference:
@@ -344,7 +345,8 @@
       return pointerTypesAreCompatible(lcanon, rcanon);
     case Type::Reference:
       return referenceTypesAreCompatible(lcanon, rcanon);
-    case Type::Array:
+    case Type::ConstantArray:
+    case Type::VariableArray:
       return arrayTypesAreCompatible(lcanon, rcanon);
     case Type::FunctionNoProto:
     case Type::FunctionProto:
@@ -487,18 +489,16 @@
       return true;
     return false;
   }
-  return CanonicalType->getTypeClass() == Array;
+  return CanonicalType->getTypeClass() == ConstantArray ||
+         CanonicalType->getTypeClass() == VariableArray;
 }
 
 // The only variable size types are auto arrays within a function. Structures 
 // cannot contain a VLA member. They can have a flexible array member, however
 // the structure is still constant size (C99 6.7.2.1p16).
 bool Type::isConstantSizeType(ASTContext &Ctx, SourceLocation *loc) const {
-  if (const ArrayType *Ary = dyn_cast<ArrayType>(CanonicalType)) {
-    assert(Ary->getSizeExpr() && "Incomplete types don't have a size at all!");
-    // Variable Length Array?
-    return Ary->getSizeExpr()->isIntegerConstantExpr(Ctx, loc);
-  }
+  if (isa<VariableArrayType>(CanonicalType))
+    return false;
   return true;
 }
 
@@ -516,9 +516,9 @@
     // A tagged type (struct/union/enum/class) is incomplete if the decl is a
     // forward declaration, but not a full definition (C99 6.2.5p22).
     return !cast<TagType>(CanonicalType)->getDecl()->isDefinition();
-  case Array:
+  case VariableArray:
     // An array of unknown size is an incomplete type (C99 6.2.5p22).
-    return cast<ArrayType>(CanonicalType)->getSizeExpr() == 0;
+    return cast<VariableArrayType>(CanonicalType)->getSizeExpr() == 0;
   }
 }
 
@@ -689,7 +689,15 @@
   ReferenceeType.getAsStringInternal(S);
 }
 
-void ArrayType::getAsStringInternal(std::string &S) const {
+void ConstantArrayType::getAsStringInternal(std::string &S) const {
+  S += '[';
+  S += llvm::utostr_32(getSize().getZExtValue());
+  S += ']';
+  
+  getElementType().getAsStringInternal(S);
+}
+
+void VariableArrayType::getAsStringInternal(std::string &S) const {
   S += '[';
   
   if (IndexTypeQuals) {
@@ -702,9 +710,14 @@
   else if (SizeModifier == Star)
     S += '*';
   
+  if (getSizeExpr()) {
+    std::ostringstream s;
+    getSizeExpr()->printPretty(s);
+    S += s.str();
+  }
   S += ']';
   
-  ElementType.getAsStringInternal(S);
+  getElementType().getAsStringInternal(S);
 }
 
 void VectorType::getAsStringInternal(std::string &S) const {

Modified: cfe/trunk/CodeGen/CodeGenTypes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/CodeGen/CodeGenTypes.cpp?rev=41592&r1=41591&r2=41592&view=diff

==============================================================================
--- cfe/trunk/CodeGen/CodeGenTypes.cpp (original)
+++ cfe/trunk/CodeGen/CodeGenTypes.cpp Wed Aug 29 20:06:46 2007
@@ -83,23 +83,23 @@
     return llvm::PointerType::get(ConvertType(R.getReferenceeType()));
   }
     
-  case Type::Array: {
-    const ArrayType &A = cast<ArrayType>(Ty);
+  case Type::VariableArray: {
+    const VariableArrayType &A = cast<VariableArrayType>(Ty);
     assert(A.getSizeModifier() == ArrayType::Normal &&
            A.getIndexTypeQualifier() == 0 &&
            "FIXME: We only handle trivial array types so far!");
-    
-    llvm::APSInt Size(32);
     if (A.getSizeExpr() == 0) {
       // int X[] -> [0 x int]
       return llvm::ArrayType::get(ConvertType(A.getElementType()), 0);
-    } else if (A.getSizeExpr()->isIntegerConstantExpr(Size, Context)) {
-      const llvm::Type *EltTy = ConvertType(A.getElementType());
-      return llvm::ArrayType::get(EltTy, Size.getZExtValue());
     } else {
       assert(0 && "FIXME: VLAs not implemented yet!");
     }
   }
+  case Type::ConstantArray: {
+    const ConstantArrayType &A = cast<ConstantArrayType>(Ty);
+    const llvm::Type *EltTy = ConvertType(A.getElementType());
+    return llvm::ArrayType::get(EltTy, A.getSize().getZExtValue());
+  }
   case Type::OCUVector:
   case Type::Vector: {
     const VectorType &VT = cast<VectorType>(Ty);

Modified: cfe/trunk/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Sema/SemaDecl.cpp?rev=41592&r1=41591&r2=41592&view=diff

==============================================================================
--- cfe/trunk/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/Sema/SemaDecl.cpp Wed Aug 29 20:06:46 2007
@@ -29,28 +29,38 @@
 // a constant expression of type int with a value greater than zero.
 bool Sema::VerifyConstantArrayType(const ArrayType *Array,
                                    SourceLocation DeclLoc) { 
-  const Expr *Size = Array->getSizeExpr();
-  if (Size == 0) return false;  // incomplete type.
-  
-  if (!Size->getType()->isIntegerType()) {
-    Diag(Size->getLocStart(), diag::err_array_size_non_int, 
-         Size->getType().getAsString(), Size->getSourceRange());
+  if (const VariableArrayType *VLA = dyn_cast<VariableArrayType>(Array)) {
+    Expr *Size = VLA->getSizeExpr();
+    if (Size == 0)
+      return false; // incomplete type.
+  
+    if (!Size->getType()->isIntegerType()) {
+      Diag(Size->getLocStart(), diag::err_array_size_non_int, 
+           Size->getType().getAsString(), Size->getSourceRange());
+      return false;
+    }
+    // FIXME: I don't think this is needed. It remains to keep test
+    // builtin_classify_type() happy...will revisit soon (today is 8/29/07:-)
+    SourceLocation Loc;
+    llvm::APSInt SizeVal(32);
+    if (!Size->isIntegerConstantExpr(SizeVal, Context, &Loc)) {
+      // FIXME: This emits the diagnostic to enforce 6.7.2.1p8, but the message
+      // is wrong.  It is also wrong for static variables.
+      // FIXME: This is also wrong for:
+      // int sub1(int i, char *pi) { typedef int foo[i];
+      // struct bar {foo f1; int f2:3; int f3:4} *p; }
+      Diag(DeclLoc, diag::err_typecheck_illegal_vla, Size->getSourceRange());
+      return false;
+    }
     return true;
   }
+  const ConstantArrayType *CAT = dyn_cast<ConstantArrayType>(Array);  
 
-  // Verify that the size of the array is an integer constant expr.
-  SourceLocation Loc;
-  llvm::APSInt SizeVal(32);
-  if (!Size->isIntegerConstantExpr(SizeVal, Context, &Loc)) {
-    // FIXME: This emits the diagnostic to enforce 6.7.2.1p8, but the message
-    // is wrong.  It is also wrong for static variables.
-    // FIXME: This is also wrong for:
-    // int sub1(int i, char *pi) { typedef int foo[i];
-    // struct bar {foo f1; int f2:3; int f3:4} *p; }
-    Diag(DeclLoc, diag::err_typecheck_illegal_vla, Size->getSourceRange());
-    return true;
-  }
+  assert(CAT && "Sema::VerifyConstantArrayType(): Illegal array type");
   
+  llvm::APSInt SizeVal(32);
+  SizeVal = CAT->getSize();
+
   // We have a constant expression with an integer type, now make sure 
   // value greater than zero (C99 6.7.5.2p1).
   
@@ -61,13 +71,11 @@
     llvm::APSInt Zero(SizeVal.getBitWidth());
     Zero.setIsUnsigned(false);
     if (SizeVal < Zero) {
-      Diag(DeclLoc, diag::err_typecheck_negative_array_size,
-           Size->getSourceRange());
+      Diag(DeclLoc, diag::err_typecheck_negative_array_size);
       return true;
     } else if (SizeVal == 0) {
       // GCC accepts zero sized static arrays.
-      Diag(DeclLoc, diag::err_typecheck_zero_array_size, 
-           Size->getSourceRange());
+      Diag(DeclLoc, diag::err_typecheck_zero_array_size);
     }
   }
   return false;
@@ -252,6 +260,17 @@
     Diag(OldD->getLocation(), diag::err_previous_definition);
     return New;
   }
+  FileVarDecl *OldFSDecl = dyn_cast<FileVarDecl>(Old);
+  FileVarDecl *NewFSDecl = dyn_cast<FileVarDecl>(New);
+  bool OldIsTentative = false;
+  
+  if (OldFSDecl && NewFSDecl) { // C99 6.9.2
+    // Handle C "tentative" external object definitions. FIXME: finish!
+    if (!OldFSDecl->getInit() &&
+        (OldFSDecl->getStorageClass() == VarDecl::None ||
+         OldFSDecl->getStorageClass() == VarDecl::Static))
+      OldIsTentative = true;
+  }
   // Verify the types match.
   if (Old->getCanonicalType() != New->getCanonicalType()) {
     Diag(New->getLocation(), diag::err_redefinition, New->getName());

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=41592&r1=41591&r2=41592&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Aug 29 20:06:46 2007
@@ -31,7 +31,7 @@
   llvm::FoldingSet<ComplexType> ComplexTypes;
   llvm::FoldingSet<PointerType> PointerTypes;
   llvm::FoldingSet<ReferenceType> ReferenceTypes;
-  llvm::FoldingSet<ArrayType> ArrayTypes;
+  llvm::FoldingSet<ConstantArrayType> ArrayTypes;
   llvm::FoldingSet<VectorType> VectorTypes;
   llvm::FoldingSet<FunctionTypeNoProto> FunctionTypeNoProtos;
   llvm::FoldingSet<FunctionTypeProto> FunctionTypeProtos;
@@ -77,10 +77,15 @@
   /// reference to the specified type.
   QualType getReferenceType(QualType T);
   
-  /// getArrayType - Return the unique reference to the type for an array of the
-  /// specified element type.
+  /// getArrayType - If NumElts is a constant expression, we return a unique
+  /// reference to an AST node of type ConstantArrayType. If NumElts is not
+  /// a constant expression, we return an instance of VaribleLengthArrayType.
   QualType getArrayType(QualType EltTy, ArrayType::ArraySizeModifier ASM,
                         unsigned EltTypeQuals, Expr *NumElts);
+
+  /// getConstantArrayType - Return the unique reference to the type for an 
+  /// array of the specified element type.
+  QualType getConstantArrayType(QualType EltTy, const llvm::APInt &Sz);
                         
   /// getVectorType - Return the unique reference to a vector type of
   /// the specified element type and size. VectorType must be a built-in type.

Modified: cfe/trunk/include/clang/AST/Type.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=41592&r1=41591&r2=41592&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Wed Aug 29 20:06:46 2007
@@ -16,6 +16,7 @@
 
 #include "llvm/Support/Casting.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/APSInt.h"
 
 using llvm::isa;
 using llvm::cast;
@@ -195,7 +196,9 @@
 class Type {
 public:
   enum TypeClass {
-    Builtin, Complex, Pointer, Reference, Array, Vector, OCUVector,
+    Builtin, Complex, Pointer, Reference, 
+    ConstantArray, VariableArray, 
+    Vector, OCUVector,
     FunctionNoProto, FunctionProto,
     TypeName, Tagged, 
     TypeOfExp, TypeOfTyp // GNU typeof extension.
@@ -430,15 +433,58 @@
 
 /// ArrayType - C99 6.7.5.2 - Array Declarators.
 ///
-class ArrayType : public Type, public llvm::FoldingSetNode {
+class ArrayType : public Type {
 public:
   /// ArraySizeModifier - Capture whether this is a normal array (e.g. int X[4])
   /// an array with a static size (e.g. int X[static 4]), or with a star size
-  /// (e.g. int X[*]).
+  /// (e.g. int X[*]). FIXME: consider moving this down to VLAArayType.
   enum ArraySizeModifier {
     Normal, Static, Star
   };
 private:
+  /// ElementType - The element type of the array.
+  QualType ElementType;
+protected:
+  ArrayType(TypeClass tc, QualType et, QualType can)
+    : Type(tc, can), ElementType(et) {}
+  friend class ASTContext;  // ASTContext creates these.
+public:
+  QualType getElementType() const { return ElementType; }
+  
+  static bool classof(const Type *T) {
+    return T->getTypeClass() == ConstantArray ||
+           T->getTypeClass() == VariableArray;
+  }
+  static bool classof(const ArrayType *) { return true; }
+};
+
+class ConstantArrayType : public ArrayType, public llvm::FoldingSetNode {
+  llvm::APInt Size; // Allows us to unique the type.
+  
+  ConstantArrayType(QualType et, QualType can, llvm::APInt sz)
+    : ArrayType(ConstantArray, et, can), Size(sz) {}
+  friend class ASTContext;  // ASTContext creates these.
+public:
+  llvm::APInt getSize() const { return Size; }
+
+  virtual void getAsStringInternal(std::string &InnerString) const;
+  
+  void Profile(llvm::FoldingSetNodeID &ID) {
+    Profile(ID, getElementType(), getSize());
+  }
+  static void Profile(llvm::FoldingSetNodeID &ID, QualType ET,
+                      llvm::APInt ArraySize) {
+    ID.AddPointer(ET.getAsOpaquePtr());
+    ID.AddInteger(ArraySize.getZExtValue());
+  }
+  static bool classof(const Type *T) { 
+    return T->getTypeClass() == ConstantArray; 
+  }
+  static bool classof(const ConstantArrayType *) { return true; }
+};
+
+// FIXME: VariableArrayType's aren't uniqued (since expressions aren't).
+class VariableArrayType : public ArrayType {
   /// NOTE: These fields are packed into the bitfields space in the Type class.
   ArraySizeModifier SizeModifier : 2;
   
@@ -446,43 +492,26 @@
   /// 'int X[static restrict 4]'.
   unsigned IndexTypeQuals : 3;
   
-  /// ElementType - The element type of the array.
-  QualType ElementType;
-  
-  /// SizeExpr - The size is either a constant or assignment expression (for 
-  /// Variable Length Arrays). VLA's are only permitted within a function block. 
+  /// SizeExpr - An assignment expression. VLA's are only permitted within 
+  /// a function block. 
   Expr *SizeExpr;
   
-  ArrayType(QualType et, ArraySizeModifier sm, unsigned tq, QualType can,
-            Expr *e)
-    : Type(Array, can), SizeModifier(sm), IndexTypeQuals(tq), ElementType(et),
-      SizeExpr(e) {}
+  VariableArrayType(QualType et, ArraySizeModifier sm, unsigned tq, 
+                          QualType can, Expr *e)
+    : ArrayType(VariableArray, et, can), 
+      SizeModifier(sm), IndexTypeQuals(tq), SizeExpr(e) {}
   friend class ASTContext;  // ASTContext creates these.
 public:
-    
-  QualType getElementType() const { return ElementType; }
   ArraySizeModifier getSizeModifier() const { return SizeModifier; }
   unsigned getIndexTypeQualifier() const { return IndexTypeQuals; }
   Expr *getSizeExpr() const { return SizeExpr; }
   
   virtual void getAsStringInternal(std::string &InnerString) const;
   
-  void Profile(llvm::FoldingSetNodeID &ID) {
-    Profile(ID, getSizeModifier(), getIndexTypeQualifier(), getElementType(),
-            getSizeExpr());
-  }
-  static void Profile(llvm::FoldingSetNodeID &ID,
-                      ArraySizeModifier SizeModifier,
-                      unsigned IndexTypeQuals, QualType ElementType,
-                      Expr *SizeExpr) {
-    ID.AddInteger(SizeModifier);
-    ID.AddInteger(IndexTypeQuals);
-    ID.AddPointer(ElementType.getAsOpaquePtr());
-    ID.AddPointer(SizeExpr);
+  static bool classof(const Type *T) { 
+    return T->getTypeClass() == VariableArray; 
   }
-  
-  static bool classof(const Type *T) { return T->getTypeClass() == Array; }
-  static bool classof(const ArrayType *) { return true; }
+  static bool classof(const VariableArrayType *) { return true; }
 };
 
 /// VectorType - GCC generic vector type. This type is created using





More information about the cfe-commits mailing list