<rdar://problem/10796651> Introduce configuration macros into module maps.

Configuration macros are macros that are intended to alter how a
module works, such that we need to build different module variants
for different values of these macros. A module can declare its
configuration macros, in which case we will complain if the definition
of a configation macro on the command line (or lack thereof) differs
from the current preprocessor state at the point where the module is
imported. This should eliminate some surprises when enabling modules,
because "#define CONFIG_MACRO ..." followed by "#include
<module/header.h>" would silently ignore the CONFIG_MACRO setting. At
least it will no longer be silent about it.

Configuration macros are eventually intended to help reduce the number
of module variants that need to be built. When the list of
configuration macros for a module is exhaustive, we only need to
consider the settings for those macros when building/finding the
module, which can help isolate modules for various project-specific -D
flags that should never affect how modules are build (but currently do).



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

13 files changed, 283 insertions, 11 deletions

diff --git a/include/clang/Basic/DiagnosticFrontendKinds.td b/include/clang/Basic/DiagnosticFrontendKinds.td
index 230a6d3dcc..111622e0fe 100644
--- a/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -137,5 +137,11 @@ def warn_missing_submodule : Warning<"missing submodule '%0'">,
 def err_module_map_temp_file : Error<
   "unable to write temporary module map file '%0'">, DefaultFatal;
 def err_module_unavailable : Error<"module '%0' requires feature '%1'">;
-
+def warn_module_config_macro_undef : Warning<
+  "%select{definition|#undef}0 of configuration macro '%1' has no effect on "
+  "the import of '%2'; pass '%select{-D%1=...|-U%1}0' on the command line "
+  "to configure the module">,
+  InGroup<ConfigMacros>;
+def note_module_def_undef_here : Note<
+  "macro was %select{defined|#undef'd}0 here">;
 }
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 0f127bf2e2..9af83b10a7 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -47,6 +47,7 @@ def CastAlign : DiagGroup<"cast-align">;
 def : DiagGroup<"cast-qual">;
 def : DiagGroup<"char-align">;
 def Comment : DiagGroup<"comment">;
+def ConfigMacros : DiagGroup<"config-macros">;
 def : DiagGroup<"ctor-dtor-privacy">;
 def GNUDesignator : DiagGroup<"gnu-designator">;
 
diff --git a/include/clang/Basic/DiagnosticLexKinds.td b/include/clang/Basic/DiagnosticLexKinds.td
index 7f66a87d61..80c52492f4 100644
--- a/include/clang/Basic/DiagnosticLexKinds.td
+++ b/include/clang/Basic/DiagnosticLexKinds.td
@@ -532,6 +532,10 @@ def err_mmap_export_module_id : Error<
   "expected an exported module name or '*'">;
 def err_mmap_expected_library_name : Error<
   "expected %select{library|framework}0 name as a string">;
+def err_mmap_config_macro_submodule : Error<
+  "configuration macros are only allowed on top-level modules">;
+def err_mmap_expected_config_macro : Error<
+  "expected configuration macro name after ','">;
 def err_mmap_missing_module_unqualified : Error<
   "no module named '%0' visible from '%1'">;
 def err_mmap_missing_module_qualified : Error<
diff --git a/include/clang/Basic/Module.h b/include/clang/Basic/Module.h
index 5f081e0f56..2add58019f 100644
--- a/include/clang/Basic/Module.h
+++ b/include/clang/Basic/Module.h
@@ -119,7 +119,14 @@ public:
   /// \brief Whether, when inferring submodules, the inferr submodules should
   /// export all modules they import (e.g., the equivalent of "export *").
   unsigned InferExportWildcard : 1;
-  
+
+  /// \brief Whether the set of configuration macros is exhaustive.
+  ///
+  /// When the set of configuration macros is exhaustive, meaning
+  /// that no identifier not in this list should affect how the module is
+  /// built.
+  unsigned ConfigMacrosExhaustive : 1;
+
   /// \brief Describes the visibility of the various names within a
   /// particular module.
   enum NameVisibilityKind {
@@ -190,6 +197,10 @@ public:
   /// an entity from this module is used.
   llvm::SmallVector<LinkLibrary, 2> LinkLibraries;
 
+  /// \brief The set of "configuration macros", which are macros that
+  /// (intentionally) change how this module is built.
+  std::vector<std::string> ConfigMacros;
+
   /// \brief Construct a top-level module.
   explicit Module(StringRef Name, SourceLocation DefinitionLoc,
                   bool IsFramework)
@@ -197,7 +208,8 @@ public:
       IsAvailable(true), IsFromModuleFile(false), IsFramework(IsFramework), 
       IsExplicit(false), IsSystem(false),
       InferSubmodules(false), InferExplicitSubmodules(false),
-      InferExportWildcard(false), NameVisibility(Hidden) { }
+      InferExportWildcard(false), ConfigMacrosExhaustive(false),
+      NameVisibility(Hidden) { }
   
   /// \brief Construct a new module or submodule.
   Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent, 
@@ -328,7 +340,7 @@ public:
   ///
   /// \returns The submodule if found, or NULL otherwise.
   Module *findSubmodule(StringRef Name) const;
-  
+
   typedef std::vector<Module *>::iterator submodule_iterator;
   typedef std::vector<Module *>::const_iterator submodule_const_iterator;
   
diff --git a/include/clang/Serialization/ASTBitCodes.h b/include/clang/Serialization/ASTBitCodes.h
index 93c5c073d5..2669d8e6d7 100644
--- a/include/clang/Serialization/ASTBitCodes.h
+++ b/include/clang/Serialization/ASTBitCodes.h
@@ -609,7 +609,9 @@ namespace clang {
       /// from this submodule.
       SUBMODULE_EXCLUDED_HEADER = 9,
       /// \brief Specifies a library or framework to link against.
-      SUBMODULE_LINK_LIBRARY = 10
+      SUBMODULE_LINK_LIBRARY = 10,
+      /// \brief Specifies a configuration macro for this module.
+      SUBMODULE_CONFIG_MACRO = 11
     };
 
     /// \brief Record types used within a comments block.
diff --git a/lib/Basic/Module.cpp b/lib/Basic/Module.cpp
index f074391098..019d04732b 100644
--- a/lib/Basic/Module.cpp
+++ b/lib/Basic/Module.cpp
@@ -279,7 +279,19 @@ void Module::print(raw_ostream &OS, unsigned Indent) const {
     OS.write_escaped(UmbrellaDir->getName());
     OS << "\"\n";    
   }
-  
+
+  if (!ConfigMacros.empty() || ConfigMacrosExhaustive) {
+    OS.indent(Indent + 2);
+    OS << "config_macros ";
+    if (ConfigMacrosExhaustive)
+      OS << "[exhausive]";
+    for (unsigned I = 0, N = ConfigMacros.size(); I != N; ++I) {
+      if (I)
+        OS << ", ";
+      OS << ConfigMacros[I];
+    }
+  }
+
   for (unsigned I = 0, N = Headers.size(); I != N; ++I) {
     OS.indent(Indent + 2);
     OS << "header \"";
diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index d5c1a27ec8..a20e7d7ed0 100644
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -905,6 +905,96 @@ static void compileModule(CompilerInstance &ImportingInstance,
   }
 }
 
+/// \brief Diagnose differences between the current definition of the given
+/// configuration macro and the definition provided on the command line.
+static void checkConfigMacro(Preprocessor &PP, StringRef ConfigMacro,
+                             Module *Mod, SourceLocation ImportLoc) {
+  IdentifierInfo *Id = PP.getIdentifierInfo(ConfigMacro);
+  SourceManager &SourceMgr = PP.getSourceManager();
+  
+  // If this identifier has never had a macro definition, then it could
+  // not have changed.
+  if (!Id->hadMacroDefinition())
+    return;
+
+  // If this identifier does not currently have a macro definition,
+  // check whether it had one on the command line.
+  if (!Id->hasMacroDefinition()) {
+    MacroDirective *UndefMD = PP.getMacroDirectiveHistory(Id);
+    for (MacroDirective *MD = UndefMD; MD; MD = MD->getPrevious()) {
+
+      FileID FID = SourceMgr.getFileID(MD->getLocation());
+      if (FID.isInvalid())
+        continue;
+
+      const llvm::MemoryBuffer *Buffer = SourceMgr.getBuffer(FID);
+      if (!Buffer)
+        continue;
+
+      // We only care about the predefines buffer.
+      if (!StringRef(Buffer->getBufferIdentifier()).equals("<built-in>"))
+        continue;
+
+      // This macro was defined on the command line, then #undef'd later.
+      // Complain.
+      PP.Diag(ImportLoc, diag::warn_module_config_macro_undef)
+        << true << ConfigMacro << Mod->getFullModuleName();
+      if (UndefMD->getUndefLoc().isValid())
+        PP.Diag(UndefMD->getUndefLoc(), diag::note_module_def_undef_here)
+          << true;
+      return;
+    }
+
+    // Okay: no definition in the predefines buffer.
+    return;
+  }
+
+  // This identifier has a macro definition. Check whether we had a definition
+  // on the command line.
+  MacroDirective *DefMD = PP.getMacroDirective(Id);
+  MacroDirective *PredefinedMD = 0;
+  for (MacroDirective *MD = DefMD; MD; MD = MD->getPrevious()) {
+    FileID FID = SourceMgr.getFileID(MD->getLocation());
+    if (FID.isInvalid())
+      continue;
+
+    const llvm::MemoryBuffer *Buffer = SourceMgr.getBuffer(FID);
+    if (!Buffer)
+      continue;
+
+    // We only care about the predefines buffer.
+    if (!StringRef(Buffer->getBufferIdentifier()).equals("<built-in>"))
+      continue;
+
+    PredefinedMD = MD;
+    break;
+  }
+
+  // If there was no definition for this macro in the predefines buffer,
+  // complain.
+  if (!PredefinedMD ||
+      (!PredefinedMD->getLocation().isValid() &&
+       PredefinedMD->getUndefLoc().isValid())) {
+    PP.Diag(ImportLoc, diag::warn_module_config_macro_undef)
+      << false << ConfigMacro << Mod->getFullModuleName();
+    PP.Diag(DefMD->getLocation(), diag::note_module_def_undef_here)
+      << false;
+    return;
+  }
+
+  // If the current macro definition is the same as the predefined macro
+  // definition, it's okay.
+  if (DefMD == PredefinedMD ||
+      DefMD->getInfo()->isIdenticalTo(*PredefinedMD->getInfo(), PP))
+    return;
+
+  // The macro definitions differ.
+  PP.Diag(ImportLoc, diag::warn_module_config_macro_undef)
+    << false << ConfigMacro << Mod->getFullModuleName();
+  PP.Diag(DefMD->getLocation(), diag::note_module_def_undef_here)
+    << false;
+}
+
 ModuleLoadResult
 CompilerInstance::loadModule(SourceLocation ImportLoc,
                              ModuleIdPath Path,
@@ -1177,7 +1267,14 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
 
     ModuleManager->makeModuleVisible(Module, Visibility, ImportLoc);
   }
-  
+
+  // Check for any configuration macros that have changed.
+  clang::Module *TopModule = Module->getTopLevelModule();
+  for (unsigned I = 0, N = TopModule->ConfigMacros.size(); I != N; ++I) {
+    checkConfigMacro(getPreprocessor(), TopModule->ConfigMacros[I],
+                     Module, ImportLoc);
+  }
+
   // If this module import was due to an inclusion directive, create an 
   // implicit import declaration to capture it in the AST.
   if (IsInclusionDirective && hasASTContext()) {
diff --git a/lib/Lex/ModuleMap.cpp b/lib/Lex/ModuleMap.cpp
index 71a98e2152..9cea5aace7 100644
--- a/lib/Lex/ModuleMap.cpp
+++ b/lib/Lex/ModuleMap.cpp
@@ -643,6 +643,7 @@ namespace clang {
   struct MMToken {
     enum TokenKind {
       Comma,
+      ConfigMacros,
       EndOfFile,
       HeaderKeyword,
       Identifier,
@@ -687,10 +688,13 @@ namespace clang {
 
   /// \brief The set of attributes that can be attached to a module.
   struct Attributes {
-    Attributes() : IsSystem() { }
+    Attributes() : IsSystem(), IsExhaustive() { }
 
     /// \brief Whether this is a system module.
     unsigned IsSystem : 1;
+
+    /// \brief Whether this is an exhaustive set of configuration macros.
+    unsigned IsExhaustive : 1;
   };
   
 
@@ -739,6 +743,7 @@ namespace clang {
     void parseUmbrellaDirDecl(SourceLocation UmbrellaLoc);
     void parseExportDecl();
     void parseLinkDecl();
+    void parseConfigMacros();
     void parseInferredModuleDecl(bool Framework, bool Explicit);
     bool parseOptionalAttributes(Attributes &Attrs);
 
@@ -776,11 +781,12 @@ retry:
     Tok.StringData = LToken.getRawIdentifierData();
     Tok.StringLength = LToken.getLength();
     Tok.Kind = llvm::StringSwitch<MMToken::TokenKind>(Tok.getString())
-                 .Case("header", MMToken::HeaderKeyword)
+                 .Case("config_macros", MMToken::ConfigMacros)
                  .Case("exclude", MMToken::ExcludeKeyword)
                  .Case("explicit", MMToken::ExplicitKeyword)
                  .Case("export", MMToken::ExportKeyword)
                  .Case("framework", MMToken::FrameworkKeyword)
+                 .Case("header", MMToken::HeaderKeyword)
                  .Case("link", MMToken::LinkKeyword)
                  .Case("module", MMToken::ModuleKeyword)
                  .Case("requires", MMToken::RequiresKeyword)
@@ -937,7 +943,9 @@ namespace {
     /// \brief An unknown attribute.
     AT_unknown,
     /// \brief The 'system' attribute.
-    AT_system
+    AT_system,
+    /// \brief The 'exhaustive' attribute.
+    AT_exhaustive
   };
 }
 
@@ -1094,7 +1102,11 @@ void ModuleMapParser::parseModuleDecl() {
     case MMToken::RBrace:
       Done = true;
       break;
-        
+
+    case MMToken::ConfigMacros:
+      parseConfigMacros();
+      break;
+
     case MMToken::ExplicitKeyword:
     case MMToken::FrameworkKeyword:
     case MMToken::ModuleKeyword:
@@ -1489,6 +1501,59 @@ void ModuleMapParser::parseLinkDecl() {
                                                             IsFramework));
 }
 
+/// \brief Parse a configuration macro declaration.
+///
+///   module-declaration:
+///     'config_macros' attributes[opt] config-macro-list?
+///
+///   config-macro-list:
+///     identifier (',' identifier)?
+void ModuleMapParser::parseConfigMacros() {
+  assert(Tok.is(MMToken::ConfigMacros));
+  SourceLocation ConfigMacrosLoc = consumeToken();
+
+  // Only top-level modules can have configuration macros.
+  if (ActiveModule->Parent) {
+    Diags.Report(ConfigMacrosLoc, diag::err_mmap_config_macro_submodule);
+  }
+
+  // Parse the optional attributes.
+  Attributes Attrs;
+  parseOptionalAttributes(Attrs);
+  if (Attrs.IsExhaustive && !ActiveModule->Parent) {
+    ActiveModule->ConfigMacrosExhaustive = true;
+  }
+
+  // If we don't have an identifier, we're done.
+  if (!Tok.is(MMToken::Identifier))
+    return;
+
+  // Consume the first identifier.
+  if (!ActiveModule->Parent) {
+    ActiveModule->ConfigMacros.push_back(Tok.getString().str());
+  }
+  consumeToken();
+
+  do {
+    // If there's a comma, consume it.
+    if (!Tok.is(MMToken::Comma))
+      break;
+    consumeToken();
+
+    // We expect to see a macro name here.
+    if (!Tok.is(MMToken::Identifier)) {
+      Diags.Report(Tok.getLocation(), diag::err_mmap_expected_config_macro);
+      break;
+    }
+
+    // Consume the macro name.
+    if (!ActiveModule->Parent) {
+      ActiveModule->ConfigMacros.push_back(Tok.getString().str());
+    }
+    consumeToken();
+  } while (true);
+}
+
 /// \brief Parse an inferred module declaration (wildcard modules).
 ///
 ///   module-declaration:
@@ -1668,6 +1733,7 @@ bool ModuleMapParser::parseOptionalAttributes(Attributes &Attrs) {
     // Decode the attribute name.
     AttributeKind Attribute
       = llvm::StringSwitch<AttributeKind>(Tok.getString())
+          .Case("exhaustive", AT_exhaustive)
           .Case("system", AT_system)
           .Default(AT_unknown);
     switch (Attribute) {
@@ -1679,6 +1745,10 @@ bool ModuleMapParser::parseOptionalAttributes(Attributes &Attrs) {
     case AT_system:
       Attrs.IsSystem = true;
       break;
+
+    case AT_exhaustive:
+      Attrs.IsExhaustive = true;
+      break;
     }
     consumeToken();
 
@@ -1730,6 +1800,7 @@ bool ModuleMapParser::parseModuleMapFile() {
       break;
 
     case MMToken::Comma:
+    case MMToken::ConfigMacros:
     case MMToken::ExcludeKeyword:
     case MMToken::ExportKeyword:
     case MMToken::HeaderKeyword:
diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 42b5038ae9..00e5ce910b 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -3718,6 +3718,18 @@ bool ASTReader::ReadSubmoduleBlock(ModuleFile &F) {
       CurrentModule->LinkLibraries.push_back(
                                          Module::LinkLibrary(Blob, Record[0]));
       break;
+
+    case SUBMODULE_CONFIG_MACRO:
+      if (First) {
+        Error("missing submodule metadata record at beginning of block");
+        return true;
+      }
+
+      if (!CurrentModule)
+        break;
+
+      CurrentModule->ConfigMacros.push_back(Blob.str());
+      break;
     }
   }
 }
diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index 724c8cf825..08e8afdd0a 100644
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -2135,6 +2135,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // InferSubmodules...
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // InferExplicit...
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // InferExportWild...
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // ConfigMacrosExh...
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
   unsigned DefinitionAbbrev = Stream.EmitAbbrev(Abbrev);
 
@@ -2174,6 +2175,11 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));     // Name
   unsigned LinkLibraryAbbrev = Stream.EmitAbbrev(Abbrev);
 
+  Abbrev = new BitCodeAbbrev();
+  Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_CONFIG_MACRO));
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));    // Macro name
+  unsigned ConfigMacroAbbrev = Stream.EmitAbbrev(Abbrev);
+
   // Write the submodule metadata block.
   RecordData Record;
   Record.push_back(getNumberOfModules(WritingModule));
@@ -2204,6 +2210,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
     Record.push_back(Mod->InferSubmodules);
     Record.push_back(Mod->InferExplicitSubmodules);
     Record.push_back(Mod->InferExportWildcard);
+    Record.push_back(Mod->ConfigMacrosExhaustive);
     Stream.EmitRecordWithBlob(DefinitionAbbrev, Record, Mod->Name);
     
     // Emit the requirements.
@@ -2288,6 +2295,14 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
                                 Mod->LinkLibraries[I].Library);
     }
 
+    // Emit the configuration macros.
+    for (unsigned I = 0, N =  Mod->ConfigMacros.size(); I != N; ++I) {
+      Record.clear();
+      Record.push_back(SUBMODULE_CONFIG_MACRO);
+      Stream.EmitRecordWithBlob(ConfigMacroAbbrev, Record,
+                                Mod->ConfigMacros[I]);
+    }
+
     // Queue up the submodules of this module.
     for (Module::submodule_iterator Sub = Mod->submodule_begin(),
                                  SubEnd = Mod->submodule_end();
diff --git a/test/Modules/Inputs/config.h b/test/Modules/Inputs/config.h
new file mode 100644
index 0000000000..f2dfda64c3
--- /dev/null
+++ b/test/Modules/Inputs/config.h
@@ -0,0 +1,7 @@
+#ifdef WANT_FOO
+int* foo();
+#endif
+
+#ifdef WANT_BAR
+char *bar();
+#endif
diff --git a/test/Modules/Inputs/module.map b/test/Modules/Inputs/module.map
index 53f2fd65d3..479bfcfa7f 100644
--- a/test/Modules/Inputs/module.map
+++ b/test/Modules/Inputs/module.map
@@ -183,3 +183,8 @@ module cxx_inline_namespace {
 module cxx_linkage_cache {
   header "cxx-linkage-cache.h"
 }
+
+module config {
+  header "config.h"
+  config_macros [exhaustive] WANT_FOO, WANT_BAR
+}
diff --git a/test/Modules/config_macros.m b/test/Modules/config_macros.m
new file mode 100644
index 0000000000..200744d614
--- /dev/null
+++ b/test/Modules/config_macros.m
@@ -0,0 +1,28 @@
+@import config;
+
+int *test_foo() {
+  return foo();
+}
+
+char *test_bar() {
+  return bar(); // expected-warning{{implicit declaration of function 'bar' is invalid in C99}} \
+                // expected-warning{{incompatible integer to pointer conversion}}
+}
+
+#undef WANT_FOO // expected-note{{macro was #undef'd here}}
+@import config; // expected-warning{{#undef of configuration macro 'WANT_FOO' has no effect on the import of 'config'; pass '-UWANT_FOO' on the command line to configure the module}}
+
+#define WANT_FOO 2 // expected-note{{macro was defined here}}
+@import config; // expected-warning{{definition of configuration macro 'WANT_FOO' has no effect on the import of 'config'; pass '-DWANT_FOO=...' on the command line to configure the module}}
+
+#undef WANT_FOO
+#define WANT_FOO 1
+@import config; // okay
+
+#define WANT_BAR 1 // expected-note{{macro was defined here}}
+@import config; // expected-warning{{definition of configuration macro 'WANT_BAR' has no effect on the import of 'config'; pass '-DWANT_BAR=...' on the command line to configure the module}}
+
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -x objective-c -fmodules-cache-path=%t -DWANT_FOO=1 -emit-module -fmodule-name=config %S/Inputs/module.map
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -DWANT_FOO=1 %s -verify
+