[ClangImporter] Handle macros that are undefined and then redefined

lib/ClangImporter/ClangImporter.cpp

@@ -2004,6 +2004,11 @@ static bool isVisibleFromModule(const ClangModuleUnit *ModuleFilter,
     }
   }
 
+  // Macros can be "redeclared" too, by putting an equivalent definition in two
+  // different modules.
+  if (ClangNode.getAsMacro())
+    return true;
+
   return false;
 }
 

lib/ClangImporter/SwiftLookupTable.cpp

@@ -1672,6 +1672,16 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
   }
 }
 
+/// Returns the nearest parent of \p module that is marked \c explicit in its
+/// module map. If \p module is itself explicit, it is returned; if no module
+/// in the parent chain is explicit, the top-level module is returned.
+static const clang::Module *
+getExplicitParentModule(const clang::Module *module) {
+  while (!module->IsExplicit && module->Parent)
+    module = module->Parent;
+  return module;
+}
+
 void importer::addMacrosToLookupTable(SwiftLookupTable &table,
                                       NameImporter &nameImporter) {
   auto &pp = nameImporter.getClangPreprocessor();
@@ -1729,16 +1739,30 @@ void importer::addMacrosToLookupTable(SwiftLookupTable &table,
       maybeAddMacro(MD->getMacroInfo(), nullptr);
 
     } else {
+      clang::Module *currentModule = pp.getCurrentModule();
       SmallVector<clang::ModuleMacro *, 8> worklist;
-      worklist.append(moduleMacros.begin(), moduleMacros.end());
+      llvm::copy_if(moduleMacros, std::back_inserter(worklist),
+                    [currentModule](const clang::ModuleMacro *next) -> bool {
+        return next->getOwningModule()->isSubModuleOf(currentModule);
+      });
+
       while (!worklist.empty()) {
         clang::ModuleMacro *moduleMacro = worklist.pop_back_val();
-        clang::Module *owningModule = moduleMacro->getOwningModule();
-        if (!owningModule->isSubModuleOf(pp.getCurrentModule()))
-          continue;
-        worklist.append(moduleMacro->overrides_begin(),
-                        moduleMacro->overrides_end());
         maybeAddMacro(moduleMacro->getMacroInfo(), moduleMacro);
+
+        // Also visit overridden macros that are in a different explicit
+        // submodule. This isn't a perfect way to tell if these two macros are
+        // supposed to be independent, but it's close enough in practice.
+        clang::Module *owningModule = moduleMacro->getOwningModule();
+        auto *explicitParent = getExplicitParentModule(owningModule);
+        llvm::copy_if(moduleMacro->overrides(), std::back_inserter(worklist),
+                      [&](const clang::ModuleMacro *next) -> bool {
+          const clang::Module *nextModule =
+              getExplicitParentModule(next->getOwningModule());
+          if (!nextModule->isSubModuleOf(currentModule))
+            return false;
+          return nextModule != explicitParent;
+        });
       }
     }
   }

test/ClangImporter/Inputs/custom-modules/MacrosDeliberateRedefA/New.h

@@ -0,0 +1,5 @@
+#include "Old.h"
+#undef MDR_REDEF_1
+#define MDR_REDEF_1 "hello"
+#undef MDR_REDEF_2
+#define MDR_REDEF_2 "swift"

test/ClangImporter/Inputs/custom-modules/MacrosDeliberateRedefA/Old.h

@@ -0,0 +1,2 @@
+#define MDR_REDEF_1 "hello-OLDTAG"
+#define MDR_REDEF_2 "swift-OLDTAG"

test/ClangImporter/Inputs/custom-modules/MacrosDeliberateRedefA/module.modulemap

@@ -0,0 +1,8 @@
+module MacrosDeliberateRedefA {
+  module Old {
+    header "Old.h"
+  }
+  module New {
+    header "New.h"
+  }
+}

test/ClangImporter/Inputs/custom-modules/MacrosDeliberateRedefB/Newer.h

@@ -0,0 +1,4 @@
+#undef MDR_REDEF_1
+#define MDR_REDEF_1 "hello"
+#undef MDR_REDEF_2
+#define MDR_REDEF_2 "world"

test/ClangImporter/Inputs/custom-modules/MacrosDeliberateRedefB/module.modulemap

@@ -0,0 +1,5 @@
+module MacrosDeliberateRedefB {
+  module Newer {
+    header "Newer.h"
+  }
+}

test/ClangImporter/macros_redef.swift

@@ -1,9 +1,13 @@
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -import-objc-header %S/Inputs/macros_redef.h -typecheck %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -import-objc-header %S/Inputs/macros_redef.h -emit-silgen %s | %FileCheck -check-prefix=NEGATIVE %s
 
 // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -import-objc-header %S/Inputs/macros_redef.h -DCONFLICT -typecheck -verify %s
 
+// NEGATIVE-NOT: OLDTAG
+
 import MacrosRedefA
 import MacrosRedefB
+import MacrosDeliberateRedefA
+import MacrosDeliberateRedefB
 
 #if CONFLICT
 import MacrosRedefWithSubmodules
@@ -44,3 +48,18 @@ func testParallelSubmodules() {
   _ = s
 }
 
+func testDeliberateRedef() {
+  var s: String
+  s = MacrosDeliberateRedefA.MDR_REDEF_1
+  s = MacrosDeliberateRedefB.MDR_REDEF_1
+  s = MDR_REDEF_1
+
+#if CONFLICT
+  // The first two lines ought to work even when SILGen-ing, but the two
+  // definitions of MDR_REDEF_2 end up getting the same mangled name.
+  s = MacrosDeliberateRedefA.MDR_REDEF_2 // ok
+  s = MacrosDeliberateRedefB.MDR_REDEF_2 // ok
+  s = MDR_REDEF_2 // expected-error{{ambiguous use of 'MDR_REDEF_2'}}
+#endif
+}
+