]> granicus.if.org Git - clang/commitdiff
Fix the following redefinition errors submitted by Keith Bauer...
authorSteve Naroff <snaroff@apple.com>
Thu, 30 Aug 2007 01:06:46 +0000 (01:06 +0000)
committerSteve Naroff <snaroff@apple.com>
Thu, 30 Aug 2007 01:06:46 +0000 (01:06 +0000)
[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!

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

AST/ASTContext.cpp
AST/Type.cpp
CodeGen/CodeGenTypes.cpp
Sema/SemaDecl.cpp
include/clang/AST/ASTContext.h
include/clang/AST/Type.h

index a68ebb5be6517bb2b8892378eb933d268e9e38a3..57c1c44dbe31cb7d3807914d05baa39215f45860 100644 (file)
@@ -162,16 +162,14 @@ ASTContext::getTypeInfo(QualType T, SourceLocation L) {
   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 @@ QualType ASTContext::getReferenceType(QualType T) {
   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) {
index 9fca35a66e38198ea6d5d32436b4e65e090e74a5..4d1cdf84973de865eb8432b713515a83f3fa6b0d 100644 (file)
@@ -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)) {
-    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 @@ 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 {
index 08c46b54bd03c868bd9e4ea129afe3d1c8b20131..85d55844b4ed0537fb8b32ce15b3ab4c70b82afc 100644 (file)
@@ -83,23 +83,23 @@ const llvm::Type *CodeGenTypes::ConvertType(QualType T) {
     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);
index 9a781b9ce49beb96e2656d34757558d49cb42991..f1a6e78b6a8fb1b74df99f62d307874c182261b3 100644 (file)
@@ -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 (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 @@ 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,
-           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 @@ 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());
index 782b2fcfcab9120e56f5a3a8bb1901934a01ad23..372109a8f82a80c688bddfb7d9a45adb4d56aa30 100644 (file)
@@ -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
-  /// 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.
index 54cc77c55bd0d3bd3540724c303e121fd64d2f24..58b3f4af51388a295a577a8eed4072f0e89f49e4 100644 (file)
@@ -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.
-  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