r235941 - [modules] Incrementally compute the list of overridden module macros based on

Alexey Samsonov vonosmas at gmail.com
Mon Apr 27 17:23:12 PDT 2015


Hi Richard,

Looks like this broke down ASan bootstrap:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/3434/steps/check-clang%20asan/logs/stdio

On Mon, Apr 27, 2015 at 4:21 PM, Richard Smith <richard-llvm at metafoo.co.uk>
wrote:

> Author: rsmith
> Date: Mon Apr 27 18:21:38 2015
> New Revision: 235941
>
> URL: http://llvm.org/viewvc/llvm-project?rev=235941&view=rev
> Log:
> [modules] Incrementally compute the list of overridden module macros based
> on
> the active module macros at the point of definition, rather than
> reconstructing
> it from the macro history. No functionality change intended.
>
> Modified:
>     cfe/trunk/include/clang/Lex/Preprocessor.h
>     cfe/trunk/lib/Lex/PPDirectives.cpp
>     cfe/trunk/lib/Lex/PPLexerChange.cpp
>     cfe/trunk/lib/Lex/PPMacroExpansion.cpp
>     cfe/trunk/lib/Lex/Preprocessor.cpp
>
> Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=235941&r1=235940&r2=235941&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
> +++ cfe/trunk/include/clang/Lex/Preprocessor.h Mon Apr 27 18:21:38 2015
> @@ -364,57 +364,104 @@ class Preprocessor : public RefCountedBa
>    };
>    SmallVector<MacroExpandsInfo, 2> DelayedMacroExpandsCallbacks;
>
> +  /// Information about a name that has been used to define a module
> macro.
> +  struct ModuleMacroInfo {
> +    ModuleMacroInfo(MacroDirective *MD)
> +        : MD(MD), ActiveModuleMacrosGeneration(0) {}
> +
> +    /// The most recent macro directive for this identifier.
> +    MacroDirective *MD;
> +    /// The active module macros for this identifier.
> +    llvm::TinyPtrVector<ModuleMacro*> ActiveModuleMacros;
> +    /// The generation number at which we last updated ActiveModuleMacros.
> +    /// \see Preprocessor::MacroVisibilityGeneration.
> +    unsigned ActiveModuleMacrosGeneration;
> +    /// Whether this macro name is ambiguous.
> +    bool IsAmbiguous;
> +    /// The module macros that are overridden by this macro.
> +    llvm::TinyPtrVector<ModuleMacro*> OverriddenMacros;
> +  };
> +
>    /// The state of a macro for an identifier.
>    class MacroState {
> -    struct ExtInfo {
> -      ExtInfo(MacroDirective *MD) : MD(MD) {}
> -
> -      // The most recent macro directive for this identifier.
> -      MacroDirective *MD;
> -      // The module macros that are overridden by this macro.
> -      SmallVector<ModuleMacro*, 4> OverriddenMacros;
> -    };
> +    mutable llvm::PointerUnion<MacroDirective *, ModuleMacroInfo *> State;
>
> -    llvm::PointerUnion<MacroDirective *, ExtInfo *> State;
> -
> -    ExtInfo &getExtInfo(Preprocessor &PP) {
> -      auto *Ext = State.dyn_cast<ExtInfo*>();
> -      if (!Ext) {
> -        Ext = new (PP.getPreprocessorAllocator())
> -            ExtInfo(State.get<MacroDirective *>());
> -        State = Ext;
> +    ModuleMacroInfo *getModuleInfo(Preprocessor &PP, IdentifierInfo *II)
> const {
> +      // FIXME: Find a spare bit on IdentifierInfo and store a
> +      //        HasModuleMacros flag.
> +      if (!II->hasMacroDefinition() || !PP.getLangOpts().Modules ||
> +          !PP.MacroVisibilityGeneration)
> +        return nullptr;
> +
> +      auto *Info = State.dyn_cast<ModuleMacroInfo*>();
> +      if (!Info) {
> +        Info = new (PP.getPreprocessorAllocator())
> +            ModuleMacroInfo(State.get<MacroDirective *>());
> +        State = Info;
>        }
> -      return *Ext;
> +
> +      if (PP.MacroVisibilityGeneration !=
> Info->ActiveModuleMacrosGeneration)
> +        PP.updateModuleMacroInfo(II, *Info);
> +      return Info;
>      }
>
>    public:
>      MacroState() : MacroState(nullptr) {}
>      MacroState(MacroDirective *MD) : State(MD) {}
>      MacroDirective *getLatest() const {
> -      if (auto *Ext = State.dyn_cast<ExtInfo*>())
> -        return Ext->MD;
> +      if (auto *Info = State.dyn_cast<ModuleMacroInfo*>())
> +        return Info->MD;
>        return State.get<MacroDirective*>();
>      }
>      void setLatest(MacroDirective *MD) {
> -      if (auto *Ext = State.dyn_cast<ExtInfo*>())
> -        Ext->MD = MD;
> +      if (auto *Info = State.dyn_cast<ModuleMacroInfo*>())
> +        Info->MD = MD;
>        else
>          State = MD;
>      }
>
> +    bool isAmbiguous(Preprocessor &PP, IdentifierInfo *II) const {
> +      auto *Info = getModuleInfo(PP, II);
> +      return Info ? Info->IsAmbiguous : false;
> +    }
> +    ArrayRef<ModuleMacro *> getActiveModuleMacros(Preprocessor &PP,
> +                                                  IdentifierInfo *II)
> const {
> +      if (auto *Info = getModuleInfo(PP, II))
> +        return Info->ActiveModuleMacros;
> +      return None;
> +    }
> +
>      MacroDirective::DefInfo findDirectiveAtLoc(SourceLocation Loc,
>                                                 SourceManager &SourceMgr)
> const {
> +      // FIXME: Incorporate module macros into the result of this.
>        return getLatest()->findDirectiveAtLoc(Loc, SourceMgr);
>      }
>
> -    void addOverriddenMacro(Preprocessor &PP, ModuleMacro *MM) {
> -      getExtInfo(PP).OverriddenMacros.push_back(MM);
> +    void overrideActiveModuleMacros(Preprocessor &PP, IdentifierInfo *II)
> {
> +      if (auto *Info = getModuleInfo(PP, II)) {
> +        for (auto *Active : Info->ActiveModuleMacros)
> +          Info->OverriddenMacros.push_back(Active);
> +        Info->ActiveModuleMacros.clear();
> +        Info->IsAmbiguous = false;
> +      }
>      }
>      ArrayRef<ModuleMacro*> getOverriddenMacros() const {
> -      if (auto *Ext = State.dyn_cast<ExtInfo*>())
> -        return Ext->OverriddenMacros;
> +      if (auto *Info = State.dyn_cast<ModuleMacroInfo*>())
> +        return Info->OverriddenMacros;
>        return None;
>      }
> +    void setOverriddenMacros(ArrayRef<ModuleMacro*> Overrides) {
> +      auto *Info = State.dyn_cast<ModuleMacroInfo*>();
> +      if (!Info) {
> +        assert(Overrides.empty() &&
> +               "have overrides but never had module macro");
> +        return;
> +      }
> +      Info->OverriddenMacros.clear();
> +      Info->OverriddenMacros.insert(Info->OverriddenMacros.end(),
> +                                    Overrides.begin(), Overrides.end());
> +      Info->ActiveModuleMacrosGeneration = 0;
> +    }
>    };
>
>    typedef llvm::DenseMap<const IdentifierInfo *, MacroState> MacroMap;
> @@ -435,8 +482,13 @@ class Preprocessor : public RefCountedBa
>      Module *M;
>      /// The location at which the module was included.
>      SourceLocation ImportLoc;
> +
> +    struct SavedMacroInfo {
> +      MacroDirective *Latest;
> +      llvm::TinyPtrVector<ModuleMacro*> Overridden;
> +    };
>      /// The macros that were visible before we entered the module.
> -    MacroMap Macros;
> +    llvm::DenseMap<const IdentifierInfo*, SavedMacroInfo> Macros;
>
>      // FIXME: VisibleModules?
>      // FIXME: CounterValue?
> @@ -447,6 +499,10 @@ class Preprocessor : public RefCountedBa
>    void EnterSubmodule(Module *M, SourceLocation ImportLoc);
>    void LeaveSubmodule();
>
> +  /// Update the set of active module macros and ambiguity flag for a
> module
> +  /// macro name.
> +  void updateModuleMacroInfo(IdentifierInfo *II, ModuleMacroInfo &Info);
> +
>    /// The set of known macros exported from modules.
>    llvm::FoldingSet<ModuleMacro> ModuleMacros;
>
> @@ -455,6 +511,10 @@ class Preprocessor : public RefCountedBa
>    llvm::DenseMap<const IdentifierInfo *,
> llvm::TinyPtrVector<ModuleMacro*>>
>        LeafModuleMacros;
>
> +  /// The generation number for module macros. Incremented each time the
> set
> +  /// of modules with visible macros changes.
> +  unsigned MacroVisibilityGeneration;
> +
>    /// \brief Macros that we want to warn because they are not used at the
> end
>    /// of the translation unit.
>    ///
>
> Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=235941&r1=235940&r2=235941&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Apr 27 18:21:38 2015
> @@ -1693,6 +1693,7 @@ void Preprocessor::HandleIncludeDirectiv
>      ModuleLoadResult Imported
>        = TheModuleLoader.loadModule(IncludeTok.getLocation(), Path,
> Visibility,
>                                     /*IsIncludeDirective=*/true);
> +    ++MacroVisibilityGeneration;
>      assert((Imported == nullptr || Imported ==
> SuggestedModule.getModule()) &&
>             "the imported module is different than the suggested one");
>
>
> Modified: cfe/trunk/lib/Lex/PPLexerChange.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPLexerChange.cpp?rev=235941&r1=235940&r2=235941&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPLexerChange.cpp (original)
> +++ cfe/trunk/lib/Lex/PPLexerChange.cpp Mon Apr 27 18:21:38 2015
> @@ -616,7 +616,13 @@ void Preprocessor::EnterSubmodule(Module
>    auto &Info = BuildingSubmoduleStack.back();
>    // Copy across our macros and start the submodule with the current
> state.
>    // FIXME: We should start each submodule with just the predefined
> macros.
> -  Info.Macros = Macros;
> +  for (auto &M : Macros) {
> +    BuildingSubmoduleInfo::SavedMacroInfo SMI;
> +    SMI.Latest = M.second.getLatest();
> +    auto O = M.second.getOverriddenMacros();
> +    SMI.Overridden.insert(SMI.Overridden.end(), O.begin(), O.end());
> +    Info.Macros.insert(std::make_pair(M.first, SMI));
> +  }
>  }
>
>  void Preprocessor::LeaveSubmodule() {
> @@ -625,12 +631,12 @@ void Preprocessor::LeaveSubmodule() {
>    // Create ModuleMacros for any macros defined in this submodule.
>    for (auto &Macro : Macros) {
>      auto *II = const_cast<IdentifierInfo*>(Macro.first);
> -    MacroState State = Info.Macros.lookup(II);
> +    auto SavedInfo = Info.Macros.lookup(II);
>
>      // This module may have exported a new macro. If so, create a
> ModuleMacro
>      // representing that fact.
>      bool ExplicitlyPublic = false;
> -    for (auto *MD = Macro.second.getLatest(); MD != State.getLatest();
> +    for (auto *MD = Macro.second.getLatest(); MD != SavedInfo.Latest;
>           MD = MD->getPrevious()) {
>        // Skip macros defined in other submodules we #included along the
> way.
>        Module *Mod = getModuleContainingLocation(MD->getLocation());
> @@ -659,11 +665,15 @@ void Preprocessor::LeaveSubmodule() {
>        }
>      }
>
> -    // Update the macro to refer to the latest directive in the chain.
> -    State.setLatest(Macro.second.getLatest());
> +    // Restore the macro's overrides list.
> +    Macro.second.setOverriddenMacros(SavedInfo.Overridden);
> +  }
>
> -    // Restore the old macro state.
> -    Macro.second = State;
> +  if (Info.M->NameVisibility < Module::MacrosVisible) {
> +    Info.M->NameVisibility = Module::MacrosVisible;
> +    Info.M->MacroVisibilityLoc = Info.ImportLoc;
> +    ++MacroVisibilityGeneration;
> +    // FIXME: Also mark any exported macros as visible, and check for
> conflicts.
>    }
>
>    BuildingSubmoduleStack.pop_back();
>
> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=235941&r1=235940&r2=235941&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Mon Apr 27 18:21:38 2015
> @@ -50,6 +50,7 @@ void Preprocessor::appendMacroDirective(
>    auto *OldMD = StoredMD.getLatest();
>    MD->setPrevious(OldMD);
>    StoredMD.setLatest(MD);
> +  StoredMD.overrideActiveModuleMacros(*this, II);
>
>    // Set up the identifier as having associated macro history.
>    II->setHasMacroDefinition(true);
> @@ -57,43 +58,6 @@ void Preprocessor::appendMacroDirective(
>      II->setHasMacroDefinition(false);
>    if (II->isFromAST() && !MD->isImported())
>      II->setChangedSinceDeserialization();
> -
> -  // Accumulate any overridden imported macros.
> -  if (!MD->isImported() && getCurrentModule()) {
> -    Module *OwningMod = getModuleContainingLocation(MD->getLocation());
> -    if (!OwningMod)
> -      return;
> -
> -    for (auto *PrevMD = OldMD; PrevMD; PrevMD = PrevMD->getPrevious()) {
> -      Module *DirectiveMod =
> getModuleContainingLocation(PrevMD->getLocation());
> -      if (ModuleMacro *PrevMM = PrevMD->getOwningModuleMacro())
> -        StoredMD.addOverriddenMacro(*this, PrevMM);
> -      else if (ModuleMacro *PrevMM = getModuleMacro(DirectiveMod, II))
> -        // The previous macro was from another submodule that we
> #included.
> -        // FIXME: Create an import directive when importing a macro from
> a local
> -        // submodule.
> -        StoredMD.addOverriddenMacro(*this, PrevMM);
> -      else
> -        // We're still within the module defining the previous macro. We
> don't
> -        // override it.
> -        break;
> -
> -      // Stop once we leave the original macro's submodule.
> -      //
> -      // Either this submodule #included another submodule of the same
> -      // module or it just happened to be built after the other module.
> -      // In the former case, we override the submodule's macro.
> -      //
> -      // FIXME: In the latter case, we shouldn't do so, but we can't tell
> -      // these cases apart.
> -      //
> -      // FIXME: We can leave this submodule and re-enter it if it
> #includes a
> -      // header within a different submodule of the same module. In such
> cases
> -      // the overrides list will be incomplete.
> -      if (DirectiveMod != OwningMod || !PrevMD->isImported())
> -        break;
> -    }
> -  }
>  }
>
>  void Preprocessor::setLoadedMacroDirective(IdentifierInfo *II,
> @@ -157,6 +121,71 @@ ModuleMacro *Preprocessor::getModuleMacr
>    return ModuleMacros.FindNodeOrInsertPos(ID, InsertPos);
>  }
>
> +void Preprocessor::updateModuleMacroInfo(IdentifierInfo *II,
> +                                         ModuleMacroInfo &Info) {
> +  assert(Info.ActiveModuleMacrosGeneration != MacroVisibilityGeneration &&
> +         "don't need to update this macro name info");
> +  Info.ActiveModuleMacrosGeneration = MacroVisibilityGeneration;
> +
> +  auto Leaf = LeafModuleMacros.find(II);
> +  if (Leaf == LeafModuleMacros.end()) {
> +    // No imported macros at all: nothing to do.
> +    return;
> +  }
> +
> +  Info.ActiveModuleMacros.clear();
> +
> +  // Every macro that's locally overridden is overridden by a visible
> macro.
> +  llvm::DenseMap<ModuleMacro *, int> NumHiddenOverrides;
> +  for (auto *O : Info.OverriddenMacros)
> +    NumHiddenOverrides[O] = -1;
> +
> +  // Collect all macros that are not overridden by a visible macro.
> +  llvm::SmallVector<ModuleMacro *, 16> Worklist(Leaf->second.begin(),
> +                                                Leaf->second.end());
> +  while (!Worklist.empty()) {
> +    auto *MM = Worklist.pop_back_val();
> +    if (MM->getOwningModule()->NameVisibility >= Module::MacrosVisible) {
> +      // We only care about collecting definitions; undefinitions only act
> +      // to override other definitions.
> +      if (MM->getMacroInfo())
> +        Info.ActiveModuleMacros.push_back(MM);
> +    } else {
> +      for (auto *O : MM->overrides())
> +        if ((unsigned)++NumHiddenOverrides[O] ==
> O->getNumOverridingMacros())
> +          Worklist.push_back(O);
> +    }
> +  }
> +
> +  // Determine whether the macro name is ambiguous.
> +  Info.IsAmbiguous = false;
> +  MacroInfo *MI = nullptr;
> +  bool IsSystemMacro = false;
> +  if (auto *DMD = dyn_cast<DefMacroDirective>(Info.MD)) {
> +    MI = DMD->getInfo();
> +    IsSystemMacro = SourceMgr.isInSystemHeader(DMD->getLocation());
> +  }
> +  for (auto *Active : Info.ActiveModuleMacros) {
> +    auto *NewMI = Active->getMacroInfo();
> +
> +    // Before marking the macro as ambiguous, check if this is a case
> where
> +    // both macros are in system headers. If so, we trust that the system
> +    // did not get it wrong. This also handles cases where Clang's own
> +    // headers have a different spelling of certain system macros:
> +    //   #define LONG_MAX __LONG_MAX__ (clang's limits.h)
> +    //   #define LONG_MAX 0x7fffffffffffffffL (system's limits.h)
> +    //
> +    // FIXME: Remove the defined-in-system-headers check. clang's limits.h
> +    // overrides the system limits.h's macros, so there's no conflict
> here.
> +    IsSystemMacro &= Active->getOwningModule()->IsSystem;
> +    if (MI && NewMI != MI && !IsSystemMacro &&
> +        !MI->isIdenticalTo(*NewMI, *this, /*Syntactically=*/true)) {
> +      Info.IsAmbiguous = true;
> +      break;
> +    }
> +  }
> +}
> +
>  /// RegisterBuiltinMacro - Register the specified identifier in the
> identifier
>  /// table and mark it as a builtin macro to be expanded.
>  static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char
> *Name){
>
> Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=235941&r1=235940&r2=235941&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
> +++ cfe/trunk/lib/Lex/Preprocessor.cpp Mon Apr 27 18:21:38 2015
> @@ -73,7 +73,8 @@ Preprocessor::Preprocessor(IntrusiveRefC
>        ModuleImportExpectsIdentifier(false), CodeCompletionReached(0),
>        MainFileDir(nullptr), SkipMainFilePreamble(0, true),
> CurPPLexer(nullptr),
>        CurDirLookup(nullptr), CurLexerKind(CLK_Lexer),
> CurSubmodule(nullptr),
> -      Callbacks(nullptr), MacroArgCache(nullptr), Record(nullptr),
> +      Callbacks(nullptr), MacroVisibilityGeneration(0),
> +      MacroArgCache(nullptr), Record(nullptr),
>        MIChainHead(nullptr), DeserialMIChainHead(nullptr) {
>    OwnsHeaderSearch = OwnsHeaders;
>
> @@ -748,11 +749,13 @@ void Preprocessor::LexAfterModuleImport(
>    // If we have a non-empty module path, load the named module.
>    if (!ModuleImportPath.empty()) {
>      Module *Imported = nullptr;
> -    if (getLangOpts().Modules)
> +    if (getLangOpts().Modules) {
>        Imported = TheModuleLoader.loadModule(ModuleImportLoc,
>                                              ModuleImportPath,
>                                              Module::MacrosVisible,
>                                              /*IsIncludeDirective=*/false);
> +      ++MacroVisibilityGeneration;
> +    }
>      if (Callbacks && (getLangOpts().Modules ||
> getLangOpts().DebuggerSupport))
>        Callbacks->moduleImport(ModuleImportLoc, ModuleImportPath,
> Imported);
>    }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>



-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150427/213c6bd3/attachment.html>


More information about the cfe-commits mailing list