Fix unloading functions declared external "C". Refactor VisitRedeclarable.

lib/Interpreter/DeclUnloader.cpp

@@ -107,31 +107,29 @@ void DeclUnloader::removeRedeclFromChain(DeclT* R) {
   }
 }
 
-///\brief Removes given declaration from the chain of redeclarations.
-/// Rebuilds the chain and sets properly first and last redeclaration.
-/// @param[in] R - The redeclarable, its chain to be rebuilt.
-/// @param[in] DC - Remove the redecl's lookup entry from this DeclContext.
+///\brief Adds the previous declaration into the lookup map on DC.
+/// @param[in] D - The decl that is being removed.
+/// @param[in] DC - The DeclContext to add the previous declaration of D.
+///\returns the previous declaration.
 ///
-///\returns the most recent redeclaration in the new chain.
-///
-template <typename T>
-bool DeclUnloader::VisitRedeclarable(clang::Redeclarable<T>* R, DeclContext* DC) {
-  if (R->getFirstDecl() == R) {
-    // This is the only element in the chain.
-    return true;
+static Decl* handleRedelaration(Decl* D, DeclContext* DC) {
+  NamedDecl* ND = dyn_cast<NamedDecl>(D);
+  if (!ND)
+    return nullptr;
+
+  DeclarationName Name = ND->getDeclName();
+  if (Name.isEmpty())
+    return nullptr;
+
+  NamedDecl* MostRecent = ND->getMostRecentDecl();
+  NamedDecl* MostRecentNotThis = MostRecent;
+  if (MostRecentNotThis == ND) {
+    MostRecentNotThis = dyn_cast_or_null<NamedDecl>(ND->getPreviousDecl());
+    if (!MostRecentNotThis || MostRecentNotThis == ND)
+      return MostRecentNotThis;
   }
 
-  T* MostRecent = R->getMostRecentDecl();
-  T* MostRecentNotThis = MostRecent;
-  if (MostRecentNotThis == R)
-    MostRecentNotThis = R->getPreviousDecl();
-
   if (StoredDeclsMap* Map = DC->getPrimaryContext()->getLookupPtr()) {
-    // Make sure we update the lookup maps, because the removed decl might
-    // be registered in the lookup and still findable.
-    NamedDecl* ND = (T*)R;
-    DeclarationName Name = ND->getDeclName();
-    if (!Name.isEmpty()) {
     StoredDeclsMap::iterator Pos = Map->find(Name);
     if (Pos != Map->end() && !Pos->second.isNull()) {
       DeclContext::lookup_result decls = Pos->second.getLookupResult();
@@ -157,10 +155,30 @@ bool DeclUnloader::VisitRedeclarable(clang::Redeclarable<T>* R, DeclContext* DC)
       }
     }
   }
+  return MostRecentNotThis;
+}
+
+///\brief Removes given declaration from the chain of redeclarations.
+/// Rebuilds the chain and sets properly first and last redeclaration.
+/// @param[in] R - The redeclarable, its chain to be rebuilt.
+/// @param[in] DC - Remove the redecl's lookup entry from this DeclContext.
+///
+///\returns the most recent redeclaration in the new chain.
+///
+template <typename T>
+bool DeclUnloader::VisitRedeclarable(clang::Redeclarable<T>* R, DeclContext* DC) {
+  if (R->getFirstDecl() == R) {
+    // This is the only element in the chain.
+    return true;
   }
 
+  // Make sure we update the lookup maps, because the removed decl might
+  // be registered in the lookup and still findable.
+  T* MostRecentNotThis = (T*)handleRedelaration((T*)R, DC);
+
   // Set a new latest redecl.
   removeRedeclFromChain((T*)R);
+
 #ifndef NDEBUG
   // Validate redecl chain by iterating through it.
   std::set<clang::Redeclarable<T>*> CheckUnique;
@@ -169,6 +187,8 @@ bool DeclUnloader::VisitRedeclarable(clang::Redeclarable<T>* R, DeclContext* DC)
     assert(CheckUnique.insert(RD).second && "Dupe redecl chain element");
     (void)RD;
   }
+#else
+  (void)MostRecentNotThis; // templated function issues a lot -Wunused-variable
 #endif
   return true;
 }
@@ -701,23 +721,27 @@ bool DeclUnloader::VisitRedeclarable(clang::Redeclarable<T>* R, DeclContext* DC)
   bool DeclUnloader::VisitLinkageSpecDecl(LinkageSpecDecl* LSD) {
     // LinkageSpecDecl: DeclContext
 
-    // If this is an extern "C" context we have to remove any of our decls
-    // from Sema::ASTContext's record. This allows globals declared extern "C"
-    // to be redeclared subsequently
+    // Re-add any previous declarations so they are reachable throughout the
+    // translation unit. Also remove any global variables from:
+    // m_Sema->Context.getExternCContextDecl()
 
     if (LSD->isExternCContext()) {
       // Sadly ASTContext::getExternCContextDecl will create if it doesn't exist
       // Hopefully LSD->isExternCContext() means that it already does exist
-      if (ExternCContextDecl* ECD = m_Sema->Context.getExternCContextDecl()) {
-        if (StoredDeclsMap* Map = ECD->getLookupPtr()) {
-          for (Decl* D: LSD->decls()) {
-            if (NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
+      ExternCContextDecl* ECD = m_Sema->Context.getExternCContextDecl();
+      StoredDeclsMap* Map = ECD ? ECD->getLookupPtr() : nullptr;
+      
+      for (Decl* D : LSD->noload_decls()) {
+        if (NamedDecl* ND = dyn_cast<NamedDecl>(D)) {
+
+          // extern "C" linkage goes in the translation unit
+          DeclContext* DC = m_Sema->getASTContext().getTranslationUnitDecl();
+          handleRedelaration(ND, DC);
+          if (Map)
             eraseDeclFromMap(Map, ND);
         }
       }
     }
-      }
-    }
 
     bool Successful = VisitDeclContext(LSD);
     Successful &= VisitDecl(LSD);

test/CodeUnloading/ExternC.C

@@ -0,0 +1,49 @@
+//------------------------------------------------------------------------------
+// CLING - the C++ LLVM-based InterpreterG :)
+//
+// This file is dual-licensed: you can choose to license it under the University
+// of Illinois Open Source License or the GNU Lesser General Public License. See
+// LICENSE.TXT for details.
+//------------------------------------------------------------------------------
+
+// RUN: cat %s | %cling -I%S -Xclang -verify  | FileCheck %s
+// Test externCUndo
+
+.storeState "A"
+
+extern "C" int printf(const char*,...);
+
+.storeState "B"
+
+extern "C" int printf(const char*,...);
+extern "C" int printf(const char*,...);
+extern "C" int printf(const char*,...);
+extern "C" int printf(const char*,...);
+
+extern "C" {
+  int printf(const char*,...);
+  int abs(int);
+  int atexit(void (*)(void));
+  double atof(const char *);
+  void free(void* ptr);
+}
+
+.undo // extern "C" {}
+.undo // printf
+.undo // printf
+.undo // printf
+.undo // printf
+
+.compareState "B"
+
+printf("Unloaded alot\n");
+// CHECK: Unloaded alot
+
+.undo // printf()
+.undo // printf
+
+.compareState "A"
+
+printf("FAIL\n"); // expected-error@2 {{use of undeclared identifier 'printf'}}
+
+.q

test/Driver/XexternC.C

@@ -0,0 +1,17 @@
+//------------------------------------------------------------------------------
+// CLING - the C++ LLVM-based InterpreterG :)
+//
+// This file is dual-licensed: you can choose to license it under the University
+// of Illinois Open Source License or the GNU Lesser General Public License. See
+// LICENSE.TXT for details.
+//------------------------------------------------------------------------------
+
+// RUN: %cling %s | FileCheck %s
+
+extern "C" int printf(const char*,...);
+
+extern "C" void XexternC() {
+ printf("XexternC\n");
+// CHECK: XexternC
+}
+