If a module A exports a macro M, and a module B imports that macro and #undef's

it, importers of B should not see the macro. This is complicated by the fact
that A's macro could also be visible through a different path. The rules (as
hashed out on cfe-commits) are included as a documentation update in this
change.

With this, the number of regressions in libc++'s testsuite when modules are
enabled drops from 47 to 7. Those remaining 7 are also macro-related, and are
due to remaining bugs in this change (in particular, the handling of submodules
is imperfect).

llvm-svn: 202560

Author: zygoloid

clang/docs/Modules.rst

@@ -198,6 +198,40 @@ Command-line parameters
 ``-fmodule-map-file=<file>``
   Load the given module map file if a header from its directory or one of its subdirectories is loaded.
 
+Module Semantics
+================
+
+Modules are modeled as if each submodule were a separate translation unit, and a module import makes names from the other translation unit visible. Each submodule starts with a new preprocessor state and an empty translation unit.
+
+.. note::
+
+  This behavior is currently only approximated when building a module. Entities within a submodule that has already been built are visible when building later submodules in that module. This can lead to fragile modules that depend on the build order used for the submodules of the module, and should not be relied upon.
+
+As an example, in C, this implies that if two structs are defined in different submodules with the same name, those two types are distinct types (but may be *compatible* types if their definitions match. In C++, two structs defined with the same name in different submodules are the *same* type, and must be equivalent under C++'s One Definition Rule.
+
+.. note::
+
+  Clang currently only performs minimal checking for violations of the One Definition Rule.
+
+Macros
+------
+
+The C and C++ preprocessor assumes that the input text is a single linear buffer, but with modules this is not the case. It is possible to import two modules that have conflicting definitions for a macro (or where one ``#define``\s a macro and the other ``#undef``\ines it). The rules for handling macro definitions in the presence of modules are as follows:
+
+* Each definition and undefinition of a macro is considered to be a distinct entity.
+* Such entities are *visible* if they are from the current submodule or translation unit, or if they were exported from a submodule that has been imported.
+* A ``#define X`` or ``#undef X`` directive *overrides* all definitions of ``X`` that are visible at the point of the directive.
+* A ``#define`` or ``#undef`` directive is *active* if it is visible and no visible directive overrides it.
+* A set of macro directives is *consistent* if it consists of only ``#undef`` directives, or if all ``#define`` directives in the set define the macro name to the same sequence of tokens (following the usual rules for macro redefinitions).
+* If a macro name is used and the set of active directives is not consistent, the program is ill-formed. Otherwise, the (unique) meaning of the macro name is used.
+
+For example, suppose:
+
+* ``<stdio.h>`` defines a macro ``getc`` (and exports its ``#define``)
+* ``<cstdio>`` imports the ``<stdio.h>`` module and undefines the macro (and exports its ``#undef``)
+  
+The ``#undef`` overrides the ``#define``, and a source file that imports both modules *in any order* will not see ``getc`` defined as a macro.
+
 Module Map Language
 ===================
 

clang/include/clang/Basic/Module.h

@@ -160,11 +160,14 @@ class Module {
     MacrosVisible,
     /// \brief All of the names in this module are visible.
     AllVisible
-  };  
-  
-  ///\ brief The visibility of names within this particular module.
+  };
+
+  /// \brief The visibility of names within this particular module.
   NameVisibilityKind NameVisibility;
 
+  /// \brief The location at which macros within this module became visible.
+  SourceLocation MacroVisibilityLoc;
+
   /// \brief The location of the inferred submodule.
   SourceLocation InferredSubmoduleLoc;
 

clang/include/clang/Serialization/ASTReader.h

@@ -64,6 +64,7 @@ class ASTUnit; // FIXME: Layering violation and egregious hack.
 class Attr;
 class Decl;
 class DeclContext;
+class DefMacroDirective;
 class DiagnosticOptions;
 class NestedNameSpecifier;
 class CXXBaseSpecifier;
@@ -471,27 +472,31 @@ class ASTReader
   /// global submodule ID to produce a local ID.
   GlobalSubmoduleMapType GlobalSubmoduleMap;
 
+  /// \brief Information on a macro definition or undefinition that is visible
+  /// at the end of a submodule.
+  struct ModuleMacroInfo;
+
   /// \brief An entity that has been hidden.
   class HiddenName {
   public:
     enum NameKind {
       Declaration,
-      MacroVisibility
+      Macro
     } Kind;
 
   private:
     union {
       Decl *D;
-      MacroDirective *MD;
+      ModuleMacroInfo *MMI;
     };
 
     IdentifierInfo *Id;
 
   public:
     HiddenName(Decl *D) : Kind(Declaration), D(D), Id() { }
 
-    HiddenName(IdentifierInfo *II, MacroDirective *MD)
-      : Kind(MacroVisibility), MD(MD), Id(II) { }
+    HiddenName(IdentifierInfo *II, ModuleMacroInfo *MMI)
+      : Kind(Macro), MMI(MMI), Id(II) { }
 
     NameKind getKind() const { return Kind; }
 
@@ -500,15 +505,21 @@ class ASTReader
       return D;
     }
 
-    std::pair<IdentifierInfo *, MacroDirective *> getMacro() const {
-      assert(getKind() == MacroVisibility && "Hidden name is not a macro!");
-      return std::make_pair(Id, MD);
+    std::pair<IdentifierInfo *, ModuleMacroInfo *> getMacro() const {
+      assert(getKind() == Macro && "Hidden name is not a macro!");
+      return std::make_pair(Id, MMI);
     }
 };
 
+  typedef llvm::SmallDenseMap<IdentifierInfo*,
+                              ModuleMacroInfo*> HiddenMacrosMap;
+
   /// \brief A set of hidden declarations.
-  typedef SmallVector<HiddenName, 2> HiddenNames;
-  
+  struct HiddenNames {
+    SmallVector<Decl*, 2> HiddenDecls;
+    HiddenMacrosMap HiddenMacros;
+  };
+
   typedef llvm::DenseMap<Module *, HiddenNames> HiddenNamesMapType;
 
   /// \brief A mapping from each of the hidden submodules to the deserialized
@@ -564,8 +575,8 @@ class ASTReader
     ModuleFile *M;
 
     struct ModuleMacroDataTy {
-      serialization::GlobalMacroID GMacID;
-      unsigned ImportLoc;
+      uint32_t MacID;
+      serialization::SubmoduleID *Overrides;
     };
     struct PCHMacroDataTy {
       uint64_t MacroDirectivesOffset;
@@ -577,10 +588,10 @@ class ASTReader
     };
 
     PendingMacroInfo(ModuleFile *M,
-                     serialization::GlobalMacroID GMacID,
-                     SourceLocation ImportLoc) : M(M) {
-      ModuleMacroData.GMacID = GMacID;
-      ModuleMacroData.ImportLoc = ImportLoc.getRawEncoding();
+                     uint32_t MacID,
+                     serialization::SubmoduleID *Overrides) : M(M) {
+      ModuleMacroData.MacID = MacID;
+      ModuleMacroData.Overrides = Overrides;
     }
 
     PendingMacroInfo(ModuleFile *M, uint64_t MacroDirectivesOffset) : M(M) {
@@ -1253,14 +1264,14 @@ class ASTReader
   /// \param ImportLoc The location at which the import occurs.
   ///
   /// \param Complain Whether to complain about conflicting module imports.
-  void makeModuleVisible(Module *Mod, 
+  void makeModuleVisible(Module *Mod,
                          Module::NameVisibilityKind NameVisibility,
                          SourceLocation ImportLoc,
                          bool Complain);
-  
+
   /// \brief Make the names within this set of hidden names visible.
   void makeNamesVisible(const HiddenNames &Names, Module *Owner);
-  
+
   /// \brief Set the AST callbacks listener.
   void setListener(ASTReaderListener *listener) {
     Listener.reset(listener);
@@ -1687,14 +1698,27 @@ class ASTReader
   serialization::IdentifierID getGlobalIdentifierID(ModuleFile &M,
                                                     unsigned LocalID);
 
+  ModuleMacroInfo *getModuleMacro(const PendingMacroInfo &PMInfo);
+
   void resolvePendingMacro(IdentifierInfo *II, const PendingMacroInfo &PMInfo);
 
   void installPCHMacroDirectives(IdentifierInfo *II,
                                  ModuleFile &M, uint64_t Offset);
 
-  void installImportedMacro(IdentifierInfo *II, MacroDirective *MD,
+  void installImportedMacro(IdentifierInfo *II, ModuleMacroInfo *MMI,
                             Module *Owner);
 
+  typedef llvm::SmallVector<DefMacroDirective*, 1> AmbiguousMacros;
+  llvm::DenseMap<IdentifierInfo*, AmbiguousMacros> AmbiguousMacroDefs;
+
+  void
+  removeOverriddenMacros(IdentifierInfo *II, AmbiguousMacros &Ambig,
+                         llvm::ArrayRef<serialization::SubmoduleID> Overrides);
+
+  AmbiguousMacros *
+  removeOverriddenMacros(IdentifierInfo *II,
+                         llvm::ArrayRef<serialization::SubmoduleID> Overrides);
+
   /// \brief Retrieve the macro with the given ID.
   MacroInfo *getMacro(serialization::MacroID ID);
 
@@ -1875,7 +1899,7 @@ class ASTReader
   void addPendingMacroFromModule(IdentifierInfo *II,
                                  ModuleFile *M,
                                  serialization::GlobalMacroID GMacID,
-                                 SourceLocation ImportLoc);
+                                 llvm::ArrayRef<serialization::SubmoduleID>);
 
   /// \brief Add a macro to deserialize its macro directive history from a PCH.
   ///

clang/include/clang/Serialization/Module.h

@@ -171,6 +171,9 @@ class ModuleFile {
   /// If module A depends on and imports module B, both modules will have the
   /// same DirectImportLoc, but different ImportLoc (B's ImportLoc will be a
   /// source location inside module A).
+  ///
+  /// WARNING: This is largely useless. It doesn't tell you when a module was
+  /// made visible, just when the first submodule of that module was imported.
   SourceLocation DirectImportLoc;
 
   /// \brief The source location where this module was first imported.

clang/lib/Lex/MacroInfo.cpp

@@ -144,7 +144,7 @@ MacroDirective::DefInfo MacroDirective::getDefinition() {
       isPublic = VisMD->isPublic();
   }
 
-  return DefInfo();
+  return DefInfo(0, UndefLoc, !isPublic.hasValue() || isPublic.getValue());
 }
 
 const MacroDirective::DefInfo

clang/lib/Lex/PPMacroExpansion.cpp

@@ -293,11 +293,11 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
     for (MacroDirective::DefInfo PrevDef = Def.getPreviousDefinition();
          PrevDef && !PrevDef.isUndefined();
          PrevDef = PrevDef.getPreviousDefinition()) {
-      if (PrevDef.getDirective()->isAmbiguous()) {
-        Diag(PrevDef.getMacroInfo()->getDefinitionLoc(),
-             diag::note_pp_ambiguous_macro_other)
-          << Identifier.getIdentifierInfo();
-      }
+      Diag(PrevDef.getMacroInfo()->getDefinitionLoc(),
+           diag::note_pp_ambiguous_macro_other)
+        << Identifier.getIdentifierInfo();
+      if (!PrevDef.getDirective()->isAmbiguous())
+        break;
     }
   }