Fix major bugs in type resolution

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

5 files changed, 165 insertions, 32 deletions

diff --git a/include/llvm/AbstractTypeUser.h b/include/llvm/AbstractTypeUser.h
index 24c061a3bb..a36bd49e23 100644
--- a/include/llvm/AbstractTypeUser.h
+++ b/include/llvm/AbstractTypeUser.h
@@ -35,6 +35,16 @@ public:
   // after this method is invoked, OldType shall be deleted, so referencing it
   // is quite unwise.
   //
+  // Another case that is important to consider is when a type is refined, but
+  // stays in the same place in memory.  In this case OldTy will equal NewTy.
+  // This callback just notifies ATU's that the underlying structure of the type
+  // has changed... but any previously used properties are still valid.
+  //
+  // Note that it is possible to refine a type with parameters OldTy==NewTy, and
+  // OldTy is no longer abstract.  In this case, abstract type users should
+  // release their hold on a type, because it went from being abstract to
+  // concrete.
+  //
   virtual void refineAbstractType(const DerivedType *OldTy,
 				  const Type *NewTy) = 0;
 };
@@ -94,6 +104,14 @@ public:
 
   // operator-> - Allow user to dereference handle naturally...
   inline const TypeSubClass *operator->() const { return Ty; }
+
+  // removeUserFromConcrete - This function should be called when the User is
+  // notified that our type is refined... and the type is being refined to
+  // itself, which is now a concrete type.  When a type becomes concrete like
+  // this, we MUST remove ourself from the AbstractTypeUser list, even though
+  // the type is apparently concrete.
+  //
+  inline void removeUserFromConcrete();
 };
 
 
@@ -113,7 +131,13 @@ public:
   //
   virtual void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) {
     assert(get() == OldTy && "Can't refine to unknown value!");
-    PATypeHandle<TypeSC>::operator=((const TypeSC*)NewTy);
+
+    // Check to see if the type just became concrete.  If so, we have to
+    // removeUser to get off its AbstractTypeUser list
+    removeUserFromConcrete();
+
+    if (OldTy != NewTy)
+      PATypeHandle<TypeSC>::operator=((const TypeSC*)NewTy);
   }
 
   // operator= - Allow assignment to handle
diff --git a/include/llvm/DerivedTypes.h b/include/llvm/DerivedTypes.h
index fdc654d7fc..8c6b4a533b 100644
--- a/include/llvm/DerivedTypes.h
+++ b/include/llvm/DerivedTypes.h
@@ -160,7 +160,6 @@ protected:
 
   // Private ctor - Only can be created by a static member...
   ArrayType(const Type *ElType, int NumEl);
-
 public:
 
   inline const Type *getElementType() const { return ElementType; }
@@ -252,7 +251,6 @@ protected:
 
   // Private ctor - Only can be created by a static member...
   PointerType(const Type *ElType);
-
 public:
 
   inline const Type *getValueType() const { return ValueType; }
@@ -326,4 +324,10 @@ template <class TypeSubClass> void PATypeHandle<TypeSubClass>::removeUser() {
     cast<DerivedType>(Ty)->removeAbstractTypeUser(User);
 }
 
+template <class TypeSubClass>
+void PATypeHandle<TypeSubClass>::removeUserFromConcrete() {
+  if (!Ty->isAbstract())
+    cast<DerivedType>(Ty)->removeAbstractTypeUser(User);
+}
+
 #endif
diff --git a/lib/Bytecode/Reader/ConstantReader.cpp b/lib/Bytecode/Reader/ConstantReader.cpp
index fcf92fa536..ec39a9f64c 100644
--- a/lib/Bytecode/Reader/ConstantReader.cpp
+++ b/lib/Bytecode/Reader/ConstantReader.cpp
@@ -103,7 +103,8 @@ const Type *BytecodeParser::parseTypeConstant(const uchar *&Buf,
 //
 void BytecodeParser::refineAbstractType(const DerivedType *OldType, 
 					const Type *NewType) {
-  if (OldType == NewType) return;  // Type is modified, but same
+  if (OldType == NewType &&
+      OldType->isAbstract()) return;  // Type is modified, but same
 
   TypeValuesListTy::iterator I = find(MethodTypeValues.begin(), 
 				      MethodTypeValues.end(), OldType);
@@ -113,7 +114,12 @@ void BytecodeParser::refineAbstractType(const DerivedType *OldType,
 	   "Can't refine a type I don't know about!");
   }
 
-  *I = NewType;  // Update to point to new, more refined type.
+  if (OldType == NewType) {
+    assert(!OldType->isAbstract());
+    I->removeUserFromConcrete();
+  } else {
+    *I = NewType;  // Update to point to new, more refined type.
+  }
 }
 
 
diff --git a/lib/VMCore/SymbolTable.cpp b/lib/VMCore/SymbolTable.cpp
index 7a6854c240..8d6df2c787 100644
--- a/lib/VMCore/SymbolTable.cpp
+++ b/lib/VMCore/SymbolTable.cpp
@@ -25,6 +25,10 @@ SymbolTable::~SymbolTable() {
 	cast<DerivedType>(Ty)->removeAbstractTypeUser(this);
     }
   }
+
+  // TODO: FIXME: BIG ONE: This doesn't unreference abstract types for the planes
+  // that could still have entries!
+
 #ifndef NDEBUG   // Only do this in -g mode...
   bool LeftoverValues = true;
   for (iterator i = begin(); i != end(); ++i) {
@@ -173,27 +177,27 @@ void SymbolTable::insertEntry(const string &Name, const Type *VTy, Value *V) {
 // This function is called when one of the types in the type plane are refined
 void SymbolTable::refineAbstractType(const DerivedType *OldType,
 				     const Type *NewType) {
-  if (OldType == NewType) return;  // Noop, don't waste time dinking around
+  if (OldType == NewType && OldType->isAbstract())
+    return;  // Noop, don't waste time dinking around
 
-  // Get a handle to the new type plane...
-  iterator NewTypeIt = find(NewType);
-  if (NewTypeIt == super::end()) {      // If no plane exists, add one
-    NewTypeIt = super::insert(make_pair(NewType, VarMap())).first;
-
-    if (NewType->isAbstract()) {
-      cast<DerivedType>(NewType)->addAbstractTypeUser(this);
+  // Search to see if we have any values of the type oldtype.  If so, we need to
+  // move them into the newtype plane...
+  iterator TPI = find(OldType);
+  if (OldType != NewType && TPI != end()) {
+    // Get a handle to the new type plane...
+    iterator NewTypeIt = find(NewType);
+    if (NewTypeIt == super::end()) {      // If no plane exists, add one
+      NewTypeIt = super::insert(make_pair(NewType, VarMap())).first;
+      
+      if (NewType->isAbstract()) {
+        cast<DerivedType>(NewType)->addAbstractTypeUser(this);
 #if DEBUG_ABSTYPE
-      cerr << "refined to abstype: " << NewType->getDescription() <<endl;
+        cerr << "[Added] refined to abstype: "<<NewType->getDescription()<<endl;
 #endif
     }
   }
 
-  VarMap &NewPlane = NewTypeIt->second;
-
-  // Search to see if we have any values of the type oldtype.  If so, we need to
-  // move them into the newtype plane...
-  iterator TPI = find(OldType);
-  if (TPI != end()) {
+    VarMap &NewPlane = NewTypeIt->second;
     VarMap &OldPlane = TPI->second;
     while (!OldPlane.empty()) {
       pair<const string, Value*> V = *OldPlane.begin();
@@ -256,6 +260,12 @@ void SymbolTable::refineAbstractType(const DerivedType *OldType,
 
     // Remove the plane that is no longer used
     erase(TPI);
+  } else if (TPI != end()) {
+    assert(OldType == NewType);
+#if DEBUG_ABSTYPE
+    cerr << "Removing SELF type " << OldType->getDescription() << endl;
+#endif
+    OldType->removeAbstractTypeUser(this);
   }
 
   TPI = find(Type::TypeTy);
diff --git a/lib/VMCore/Type.cpp b/lib/VMCore/Type.cpp
index ba3f5e0f22..29d80fc950 100644
--- a/lib/VMCore/Type.cpp
+++ b/lib/VMCore/Type.cpp
@@ -151,6 +151,11 @@ ArrayType::ArrayType(const Type *ElType, int NumEl)
   NumElements = NumEl;
   setDerivedTypeProperties();
 }
+ArrayType::~ArrayType() {
+#ifdef DEBUG_MERGE_TYPES
+  cerr << "Destroyed type: " << getDescription() << endl;
+#endif
+}
 
 StructType::StructType(const vector<const Type*> &Types)
   : DerivedType("", StructTyID) {
@@ -166,6 +171,11 @@ PointerType::PointerType(const Type *E) : DerivedType("", PointerTyID),
 			  ValueType(PATypeHandle<Type>(E, this)) {
   setDerivedTypeProperties();
 }
+PointerType::~PointerType() {
+#ifdef DEBUG_MERGE_TYPES
+  cerr << "Destoyed type: " << getDescription() << endl;
+#endif
+}
 
 OpaqueType::OpaqueType() : DerivedType("", OpaqueTyID) {
   setAbstract(true);
@@ -381,7 +391,16 @@ public:
   // corrected.
   //
   virtual void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) {
-    if (OldTy == NewTy) return;
+    if (OldTy == NewTy) {
+      if (!OldTy->isAbstract()) {
+        // Check to see if the type just became concrete.
+        // If so, remove self from user list.
+        for (MapTy::iterator I = Map.begin(), E = Map.end(); I != E; ++I)
+          if (I->second == OldTy)
+            I->second.removeUserFromConcrete();
+      }
+      return;
+    }
 #ifdef DEBUG_MERGE_TYPES
     cerr << "Removing Old type from Tab: " << (void*)OldTy << ", "
 	 << OldTy->getDescription() << "  replacement == " << (void*)NewTy
@@ -426,9 +445,19 @@ protected:
 
   // Subclass should override this... to update self as usual
   virtual void doRefinement(const DerivedType *OldTy, const Type *NewTy) = 0;
+
+  // typeBecameConcrete - This callback occurs when a contained type refines
+  // to itself, but becomes concrete in the process.  Our subclass should remove
+  // itself from the ATU list of the specified type.
+  //
+  virtual void typeBecameConcrete(const DerivedType *Ty) = 0;
   
   virtual void refineAbstractType(const DerivedType *OldTy, const Type *NewTy) {
-    if (OldTy == NewTy) return;
+    if (OldTy == NewTy) {
+      if (!OldTy->isAbstract())
+        typeBecameConcrete(OldTy);
+      return;
+    }
     TypeMap<ValType, TypeClass> &Table = MyTable;     // Copy MyTable reference
     ValType Tmp(*(ValType*)this);                     // Copy this.
     PATypeHandle<TypeClass> OldType(Table.get(*(ValType*)this), this);
@@ -481,6 +510,13 @@ public:
       if (ArgTypes[i] == OldType) ArgTypes[i] = NewType;
   }
 
+  virtual void typeBecameConcrete(const DerivedType *Ty) {
+    if (RetTy == Ty) RetTy.removeUserFromConcrete();
+
+    for (unsigned i = 0; i < ArgTypes.size(); ++i)
+      if (ArgTypes[i] == Ty) ArgTypes[i].removeUserFromConcrete();
+  }
+
   inline bool operator<(const MethodValType &MTV) const {
     if (RetTy.get() < MTV.RetTy.get()) return true;
     if (RetTy.get() > MTV.RetTy.get()) return false;
@@ -531,6 +567,12 @@ public:
     if (ValTy == OldType) ValTy = NewType;
   }
 
+  virtual void typeBecameConcrete(const DerivedType *Ty) {
+    assert(ValTy == Ty &&
+           "Contained type became concrete but we're not using it!");
+    ValTy.removeUserFromConcrete();
+  }
+
   inline bool operator<(const ArrayValType &MTV) const {
     if (Size < MTV.Size) return true;
     return Size == MTV.Size && ValTy.get() < MTV.ValTy.get();
@@ -587,6 +629,11 @@ public:
       if (ElTypes[i] == OldType) ElTypes[i] = NewType;
   }
 
+  virtual void typeBecameConcrete(const DerivedType *Ty) {
+    for (unsigned i = 0; i < ElTypes.size(); ++i)
+      if (ElTypes[i] == Ty) ElTypes[i].removeUserFromConcrete();
+  }
+
   inline bool operator<(const StructValType &STV) const {
     return ElTypes < STV.ElTypes;
   }
@@ -631,6 +678,12 @@ public:
     if (ValTy == OldType) ValTy = NewType;
   }
 
+  virtual void typeBecameConcrete(const DerivedType *Ty) {
+    assert(ValTy == Ty &&
+           "Contained type became concrete but we're not using it!");
+    ValTy.removeUserFromConcrete();
+  }
+
   inline bool operator<(const PointerValType &MTV) const {
     return ValTy.get() < MTV.ValTy.get();
   }
@@ -675,22 +728,20 @@ void DerivedType::removeAbstractTypeUser(AbstractTypeUser *U) const {
       AbstractTypeUsers.erase(AbstractTypeUsers.begin()+i-1);
       
 #ifdef DEBUG_MERGE_TYPES
-      cerr << "  removeAbstractTypeUser[" << (void*)this << ", "
-	   << getDescription() << "][" << AbstractTypeUsers.size()
-	   << "] User = " << U << endl;
+      cerr << "  removeAbstractTypeUser<" << (void*)this << ", "
+	   << getDescription() << ">[" << i << "] User = " << U << endl;
 #endif
 
-      if (AbstractTypeUsers.empty()) {
+      if (AbstractTypeUsers.empty() && isAbstract()) {
 #ifdef DEBUG_MERGE_TYPES
-	cerr << "DELETEing unused abstract type: " << getDescription()
-	     << "[" << (void*)this << "]" << endl;
+	cerr << "DELETEing unused abstract type: <" << getDescription()
+	     << ">[" << (void*)this << "]" << endl;
 #endif
 	delete this;                  // No users of this abstract type!
       }
       return;
     }
   }
-  assert(isAbstract() && "removeAbstractTypeUser: Type not abstract!");
   assert(0 && "AbstractTypeUser not in user list!");
 }
 
@@ -746,6 +797,9 @@ void DerivedType::refineAbstractTypeTo(const Type *NewType) {
 #endif
       User->refineAbstractType(this, NewTy);
 
+      if (AbstractTypeUsers.size() == OldSize) {
+        User->refineAbstractType(this, NewTy);
+      }
       assert(AbstractTypeUsers.size() != OldSize &&
 	     "AbsTyUser did not remove self from user list!");
     }
@@ -786,6 +840,19 @@ void DerivedType::typeIsRefined() {
   }
 
   --isRefining;
+
+#ifndef _NDEBUG
+  if (!(isAbstract() || AbstractTypeUsers.empty()))
+    for (unsigned i = 0; i < AbstractTypeUsers.size(); ++i) {
+      if (AbstractTypeUsers[i] != this) {
+        // Debugging hook
+        cerr << "FOUND FAILURE\n";
+        AbstractTypeUsers[i]->refineAbstractType(this, this);
+        assert(0 && "Type became concrete,"
+               " but it still has abstract type users hanging around!");
+      }
+  }
+#endif
 }
   
 
@@ -803,6 +870,12 @@ void MethodType::refineAbstractType(const DerivedType *OldType,
        << NewType->getDescription() << "])\n";
 #endif
 
+  if (!OldType->isAbstract()) {
+    if (ResultType == OldType) ResultType.removeUserFromConcrete();
+    for (unsigned i = 0; i < ParamTys.size(); ++i)
+      if (ParamTys[i] == OldType) ParamTys[i].removeUserFromConcrete();
+  }
+
   if (OldType != NewType) {
     if (ResultType == OldType) ResultType = NewType;
 
@@ -812,10 +885,10 @@ void MethodType::refineAbstractType(const DerivedType *OldType,
 
   const MethodType *MT = MethodTypes.containsEquivalent(this);
   if (MT && MT != this) {
-    refineAbstractTypeTo(MT);          // Different type altogether...
+    refineAbstractTypeTo(MT);            // Different type altogether...
   } else {
     setDerivedTypeProperties();          // Update the name and isAbstract
-    typeIsRefined();                   // Same type, different contents...
+    typeIsRefined();                     // Same type, different contents...
   }
 }
 
@@ -832,6 +905,11 @@ void ArrayType::refineAbstractType(const DerivedType *OldType,
        << NewType->getDescription() << "])\n";
 #endif
 
+  if (!OldType->isAbstract()) {
+    assert(ElementType == OldType);
+    ElementType.removeUserFromConcrete();
+  }
+
   ElementType = NewType;
   const ArrayType *AT = ArrayTypes.containsEquivalent(this);
   if (AT && AT != this) {
@@ -854,12 +932,18 @@ void StructType::refineAbstractType(const DerivedType *OldType,
        << OldType->getDescription() << "], " << (void*)NewType << " [" 
        << NewType->getDescription() << "])\n";
 #endif
+  if (!OldType->isAbstract()) {
+    for (unsigned i = 0; i < ETypes.size(); ++i)
+      if (ETypes[i] == OldType)
+        ETypes[i].removeUserFromConcrete();
+  }
+
   if (OldType != NewType) {
     // Update old type to new type in the array...
     for (unsigned i = 0; i < ETypes.size(); ++i)
       if (ETypes[i] == OldType)
         ETypes[i] = NewType;
-  }
+  } 
 
   const StructType *ST = StructTypes.containsEquivalent(this);
   if (ST && ST != this) {
@@ -882,6 +966,11 @@ void PointerType::refineAbstractType(const DerivedType *OldType,
        << NewType->getDescription() << "])\n";
 #endif
 
+  if (!OldType->isAbstract()) {
+    assert(ValueType == OldType);
+    ValueType.removeUserFromConcrete();
+  }
+
   ValueType = NewType;
   const PointerType *PT = PointerTypes.containsEquivalent(this);