r213395 - Revert "[modules] Fix macro hiding bug exposed if:"

Ben Langmuir blangmuir at apple.com
Fri Jul 18 11:38:25 PDT 2014


Author: benlangmuir
Date: Fri Jul 18 13:38:24 2014
New Revision: 213395

URL: http://llvm.org/viewvc/llvm-project?rev=213395&view=rev
Log:
Revert "[modules] Fix macro hiding bug exposed if:"

This is breaking the system modules on Darwin, because something that
was defined and re-exported no longer is.  Might be this patch, or might
just be a really poor interaction with an existing visibility bug.

This reverts commit r213348.

Removed:
    cfe/trunk/test/Modules/Inputs/macro-hiding/a1.h
    cfe/trunk/test/Modules/Inputs/macro-hiding/a2.h
    cfe/trunk/test/Modules/Inputs/macro-hiding/b1.h
    cfe/trunk/test/Modules/Inputs/macro-hiding/b2.h
    cfe/trunk/test/Modules/Inputs/macro-hiding/c1.h
    cfe/trunk/test/Modules/Inputs/macro-hiding/module.modulemap
    cfe/trunk/test/Modules/macro-hiding.cpp
Modified:
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=213395&r1=213394&r2=213395&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Jul 18 13:38:24 2014
@@ -1366,8 +1366,7 @@ public:
                          bool Complain);
 
   /// \brief Make the names within this set of hidden names visible.
-  void makeNamesVisible(const HiddenNames &Names, Module *Owner,
-                        bool FromFinalization);
+  void makeNamesVisible(const HiddenNames &Names, Module *Owner);
 
   /// \brief Set the AST callbacks listener.
   void setListener(ASTReaderListener *listener) {
@@ -1832,7 +1831,7 @@ public:
                                  ModuleFile &M, uint64_t Offset);
 
   void installImportedMacro(IdentifierInfo *II, ModuleMacroInfo *MMI,
-                            Module *Owner, bool FromFinalization);
+                            Module *Owner);
 
   typedef llvm::TinyPtrVector<DefMacroDirective *> AmbiguousMacros;
   llvm::DenseMap<IdentifierInfo*, AmbiguousMacros> AmbiguousMacroDefs;

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=213395&r1=213394&r2=213395&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Jul 18 13:38:24 2014
@@ -1790,7 +1790,7 @@ void ASTReader::resolvePendingMacro(Iden
     // install if we make this module visible.
     HiddenNamesMap[Owner].HiddenMacros.insert(std::make_pair(II, MMI));
   } else {
-    installImportedMacro(II, MMI, Owner, /*FromFinalization*/false);
+    installImportedMacro(II, MMI, Owner);
   }
 }
 
@@ -1941,11 +1941,11 @@ ASTReader::removeOverriddenMacros(Identi
 }
 
 void ASTReader::installImportedMacro(IdentifierInfo *II, ModuleMacroInfo *MMI,
-                                     Module *Owner, bool FromFinalization) {
+                                     Module *Owner) {
   assert(II && Owner);
 
   SourceLocation ImportLoc = Owner->MacroVisibilityLoc;
-  if (ImportLoc.isInvalid() && !FromFinalization) {
+  if (ImportLoc.isInvalid()) {
     // FIXME: If we made macros from this module visible but didn't provide a
     // source location for the import, we don't have a location for the macro.
     // Use the location at which the containing module file was first imported
@@ -3320,9 +3320,7 @@ static void moveMethodToBackOfGlobalList
   }
 }
 
-void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner,
-                                 bool FromFinalization) {
-  // FIXME: Only do this if Owner->NameVisibility == AllVisible.
+void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner) {
   for (unsigned I = 0, N = Names.HiddenDecls.size(); I != N; ++I) {
     Decl *D = Names.HiddenDecls[I];
     bool wasHidden = D->Hidden;
@@ -3335,12 +3333,10 @@ void ASTReader::makeNamesVisible(const H
     }
   }
 
-  assert((FromFinalization || Owner->NameVisibility >= Module::MacrosVisible) &&
-         "nothing to make visible?");
   for (HiddenMacrosMap::const_iterator I = Names.HiddenMacros.begin(),
                                        E = Names.HiddenMacros.end();
        I != E; ++I)
-    installImportedMacro(I->first, I->second, Owner, FromFinalization);
+    installImportedMacro(I->first, I->second, Owner);
 }
 
 void ASTReader::makeModuleVisible(Module *Mod,
@@ -3374,8 +3370,7 @@ void ASTReader::makeModuleVisible(Module
     // mark them as visible.
     HiddenNamesMapType::iterator Hidden = HiddenNamesMap.find(Mod);
     if (Hidden != HiddenNamesMap.end()) {
-      makeNamesVisible(Hidden->second, Hidden->first,
-                       /*FromFinalization*/false);
+      makeNamesVisible(Hidden->second, Hidden->first);
       HiddenNamesMap.erase(Hidden);
     }
 
@@ -3902,7 +3897,7 @@ void ASTReader::finalizeForWriting() {
   for (HiddenNamesMapType::iterator Hidden = HiddenNamesMap.begin(),
                                  HiddenEnd = HiddenNamesMap.end();
        Hidden != HiddenEnd; ++Hidden) {
-    makeNamesVisible(Hidden->second, Hidden->first, /*FromFinalization*/true);
+    makeNamesVisible(Hidden->second, Hidden->first);
   }
   HiddenNamesMap.clear();
 }

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=213395&r1=213394&r2=213395&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Jul 18 13:38:24 2014
@@ -3089,17 +3089,7 @@ class ASTIdentifierTableTrait {
   }
 
   SubmoduleID getSubmoduleID(MacroDirective *MD) {
-    if (MD->getLocation().isValid())
-      return Writer.inferSubmoduleIDFromLocation(MD->getLocation());
-
-    // If we have no directive location, this macro was installed when
-    // finalizing the ASTReader.
-    if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD))
-      return DefMD->getInfo()->getOwningModuleID();
-
-    // Skip imports that only produce #undefs for now.
-    // FIXME: We should still re-export them!
-    return 0;
+    return Writer.inferSubmoduleIDFromLocation(MD->getLocation());
   }
 
 public:

Removed: cfe/trunk/test/Modules/Inputs/macro-hiding/a1.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macro-hiding/a1.h?rev=213394&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/macro-hiding/a1.h (original)
+++ cfe/trunk/test/Modules/Inputs/macro-hiding/a1.h (removed)
@@ -1 +0,0 @@
-#define assert(x)

Removed: cfe/trunk/test/Modules/Inputs/macro-hiding/a2.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macro-hiding/a2.h?rev=213394&view=auto
==============================================================================
    (empty)

Removed: cfe/trunk/test/Modules/Inputs/macro-hiding/b1.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macro-hiding/b1.h?rev=213394&view=auto
==============================================================================
    (empty)

Removed: cfe/trunk/test/Modules/Inputs/macro-hiding/b2.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macro-hiding/b2.h?rev=213394&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/macro-hiding/b2.h (original)
+++ cfe/trunk/test/Modules/Inputs/macro-hiding/b2.h (removed)
@@ -1,2 +0,0 @@
-#include "a2.h"
-#define assert(x)

Removed: cfe/trunk/test/Modules/Inputs/macro-hiding/c1.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macro-hiding/c1.h?rev=213394&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/macro-hiding/c1.h (original)
+++ cfe/trunk/test/Modules/Inputs/macro-hiding/c1.h (removed)
@@ -1,2 +0,0 @@
-#include "b1.h"
-#define assert(x)

Removed: cfe/trunk/test/Modules/Inputs/macro-hiding/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macro-hiding/module.modulemap?rev=213394&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/macro-hiding/module.modulemap (original)
+++ cfe/trunk/test/Modules/Inputs/macro-hiding/module.modulemap (removed)
@@ -1,11 +0,0 @@
-module a {
-  module a1 { header "a1.h" export * }
-  module a2 { header "a2.h" export * }
-}
-module b {
-  module b1 { header "b1.h" export * }
-  module b2 { header "b2.h" export * }
-}
-module c {
-  module c1 { header "c1.h" export * }
-}

Removed: cfe/trunk/test/Modules/macro-hiding.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/macro-hiding.cpp?rev=213394&view=auto
==============================================================================
--- cfe/trunk/test/Modules/macro-hiding.cpp (original)
+++ cfe/trunk/test/Modules/macro-hiding.cpp (removed)
@@ -1,6 +0,0 @@
-// RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/macro-hiding %s
-#include "c1.h"
-#include "b2.h"
-
-void h() { assert(true); }





More information about the cfe-commits mailing list