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!

llvm-svn: 41592

clang/AST/ASTContext.cpp

@@ -162,16 +162,14 @@ ASTContext::getTypeInfo(QualType T, SourceLocation L) {
   case Type::FunctionProto:
   default:
     assert(0 && "Incomplete types have no size!");
-  case Type::Array: {
+  case Type::VariableArray:
+    assert(0 && "VLAs not implemented yet!");
+  case Type::ConstantArray: {
+    ConstantArrayType *CAT = cast<ConstantArrayType>(T);
+    
     std::pair<uint64_t, unsigned> EltInfo = 
-      getTypeInfo(cast<ArrayType>(T)->getElementType(), L);
+      getTypeInfo(CAT->getElementType(), L);
-    
+    Size = EltInfo.first*CAT->getSize().getZExtValue();
-    // Get the size of the array.
-    llvm::APSInt Sz(32);
-    if (!cast<ArrayType>(T)->getSizeExpr()->isIntegerConstantExpr(Sz, *this))
-      assert(0 && "VLAs not implemented yet!");
-    
-    Size = EltInfo.first*Sz.getZExtValue();
     Align = EltInfo.second;
     break;
   }    
@@ -403,37 +401,55 @@ QualType ASTContext::getReferenceType(QualType T) {
   return QualType(New, 0);
 }
 
-/// getArrayType - Return the unique reference to the type for an array of the
+/// getConstantArrayType - Return the unique reference to the type for an 
-/// specified element type.
+/// array of the specified element type.
-QualType ASTContext::getArrayType(QualType EltTy,ArrayType::ArraySizeModifier ASM,
+QualType ASTContext::getConstantArrayType(QualType EltTy, 
-                                  unsigned EltTypeQuals, Expr *NumElts) {
+                                          const llvm::APInt &ArySize) {
-  // Unique array types, to guarantee there is only one array of a particular
-  // structure.
   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,
+    Canonical = getConstantArrayType(EltTy.getCanonicalType(), ArySize);
-                             NumElts);
     
     // 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) {

clang/AST/Type.cpp

@@ -43,7 +43,8 @@ bool Type::isObjectType() const {
 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 @@ bool Type::typesAreCompatible(QualType lhs, QualType rhs) {
       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 @@ bool Type::isAggregateType() const {
       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)) {
+  if (isa<VariableArrayType>(CanonicalType))
-    assert(Ary->getSizeExpr() && "Incomplete types don't have a size at all!");
+    return false;
-    // Variable Length Array?
-    return Ary->getSizeExpr()->isIntegerConstantExpr(Ctx, loc);
-  }
   return true;
 }
 
@@ -516,9 +516,9 @@ bool Type::isIncompleteType() const {
     // 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 @@ void ReferenceType::getAsStringInternal(std::string &S) const {
   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 @@ void ArrayType::getAsStringInternal(std::string &S) const {
   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 {

clang/CodeGen/CodeGenTypes.cpp

@@ -83,23 +83,23 @@ const llvm::Type *CodeGenTypes::ConvertType(QualType T) {
     return llvm::PointerType::get(ConvertType(R.getReferenceeType()));
   }
     
-  case Type::Array: {
+  case Type::VariableArray: {
-    const ArrayType &A = cast<ArrayType>(Ty);
+    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);

clang/Sema/SemaDecl.cpp

@@ -29,28 +29,38 @@ using namespace clang;
 // 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 (const VariableArrayType *VLA = dyn_cast<VariableArrayType>(Array)) {
-  if (Size == 0) return false;  // incomplete type.
+    Expr *Size = VLA->getSizeExpr();
+    if (Size == 0)
+      return false; // incomplete type.
   
-  if (!Size->getType()->isIntegerType()) {
+    if (!Size->getType()->isIntegerType()) {
-    Diag(Size->getLocStart(), diag::err_array_size_non_int, 
+      Diag(Size->getLocStart(), diag::err_array_size_non_int, 
-         Size->getType().getAsString(), Size->getSourceRange());
+           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.
+  assert(CAT && "Sema::VerifyConstantArrayType(): Illegal array type");
-  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;
-  }
   
+  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 @@ bool Sema::VerifyConstantArrayType(const ArrayType *Array,
     llvm::APSInt Zero(SizeVal.getBitWidth());
     Zero.setIsUnsigned(false);
     if (SizeVal < Zero) {
-      Diag(DeclLoc, diag::err_typecheck_negative_array_size,
+      Diag(DeclLoc, diag::err_typecheck_negative_array_size);
-           Size->getSourceRange());
       return true;
     } else if (SizeVal == 0) {
       // GCC accepts zero sized static arrays.
-      Diag(DeclLoc, diag::err_typecheck_zero_array_size, 
+      Diag(DeclLoc, diag::err_typecheck_zero_array_size);
-           Size->getSourceRange());
     }
   }
   return false;
@@ -252,6 +260,17 @@ VarDecl *Sema::MergeVarDecl(VarDecl *New, Decl *OldD) {
     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());

clang/include/clang/AST/ASTContext.h

@@ -31,7 +31,7 @@ class ASTContext {
   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 @@ public:
   /// reference to the specified type.
   QualType getReferenceType(QualType T);
   
-  /// getArrayType - Return the unique reference to the type for an array of the
+  /// getArrayType - If NumElts is a constant expression, we return a unique
-  /// specified element type.
+  /// 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.

clang/include/clang/AST/Type.h

@@ -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 @@ namespace clang {
 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 @@ public:
 
 /// 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 @@ private:
   /// 'int X[static restrict 4]'.
   unsigned IndexTypeQuals : 3;
   
-  /// ElementType - The element type of the array.
+  /// SizeExpr - An assignment expression. VLA's are only permitted within 
-  QualType ElementType;
+  /// a function block. 
-  
-  /// SizeExpr - The size is either a constant or assignment expression (for 
-  /// Variable Length Arrays). VLA's are only permitted within a function block. 
   Expr *SizeExpr;
   
-  ArrayType(QualType et, ArraySizeModifier sm, unsigned tq, QualType can,
+  VariableArrayType(QualType et, ArraySizeModifier sm, unsigned tq, 
-            Expr *e)
+                          QualType can, Expr *e)
-    : Type(Array, can), SizeModifier(sm), IndexTypeQuals(tq), ElementType(et),
+    : ArrayType(VariableArray, et, can), 
-      SizeExpr(e) {}
+      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) {
+  static bool classof(const Type *T) { 
-    Profile(ID, getSizeModifier(), getIndexTypeQualifier(), getElementType(),
+    return T->getTypeClass() == VariableArray; 
-            getSizeExpr());
   }
-  static void Profile(llvm::FoldingSetNodeID &ID,
+  static bool classof(const VariableArrayType *) { return true; }
-                      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() == Array; }
-  static bool classof(const ArrayType *) { return true; }
 };
 
 /// VectorType - GCC generic vector type. This type is created using