[ClangImporter] Handle macros that are undefined and then redefined (#12413)

This includes 'LONG_MAX' in the Darwin module, which is defined by the
system and then immediately redefined by Clang's copy of limits.h.

The general strategy here is that if two definitions of a macro in the
same module are in separate explicit submodules, we need to keep track
of both of them, but if they're in the same explicit submodule then
one is probably deliberately redefining the other. This isn't fully
correct because one explicit submodule could include another explicit
submodule, but it's a good first approximation, which we can refine if
we come across problem cases in practice.

Swift /also/ has the ability to distinguish between ModuleA.MACRO and
ModuleB.MACRO if you've imported both modules, although there's an
issue that the two of them end up getting the same mangled name if you
try to use both in the same compilation unit. (Filed
rdar://problem/34968281.) That ability is preserved with this change.

All of this will likely change if/when we switch to Clang's "local
submodule visibility" mode, which is needed for the C++ Modules TS but
is also incompatible with Apple's SDKs at the moment. In that case,
macros in two separate explicit submodules will no longer have an
'override' relationship just because they're mentioned sequentially in
the module map.

rdar://problem/34413934

Author: jrose-apple

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
+}
+