r241140 - -frewrite-includes: Rework how includes and modules are differentiated

David Blaikie dblaikie at gmail.com
Tue Jun 30 21:47:26 PDT 2015


On Tue, Jun 30, 2015 at 9:40 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> Author: bogner
> Date: Tue Jun 30 23:40:10 2015
> New Revision: 241140
>
> URL: http://llvm.org/viewvc/llvm-project?rev=241140&view=rev
> Log:
> -frewrite-includes: Rework how includes and modules are differentiated
>
> The map of FileChange structs here was storing two disjoint types of
> information:
>
> 1. A pointer to the Module that an #include directive implicitly
>    imported
>
> 2. A FileID and FileType for an included file. These would be left
>    uninitialized in the Module case.
>
> This change splits these two kinds of information into their own maps,
> which both simplifies how we access either and avoids the undefined
> behaviour we were hitting due to the uninitialized fields in the
> included file case.
>
> Mostly NFC, but fixes some errors found by self-host with ubsan.
>

Ideally it'd be nice to have those test cases reduced and committed, so
they can be found/verified/etc without a full selfhost build later on.


>
> Modified:
>     cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
>
> Modified: cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp?rev=241140&r1=241139&r2=241140&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp (original)
> +++ cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp Tue Jun 30
> 23:40:10 2015
> @@ -29,13 +29,11 @@ namespace {
>  class InclusionRewriter : public PPCallbacks {
>    /// Information about which #includes were actually performed,
>    /// created by preprocessor callbacks.
> -  struct FileChange {
> -    const Module *Mod;
> -    SourceLocation From;
> +  struct IncludedFile {
>      FileID Id;
>      SrcMgr::CharacteristicKind FileType;
> -    FileChange(SourceLocation From, const Module *Mod) : Mod(Mod),
> From(From) {
> -    }
> +    IncludedFile(FileID Id, SrcMgr::CharacteristicKind FileType)
> +        : Id(Id), FileType(FileType) {}
>    };
>    Preprocessor &PP; ///< Used to find inclusion directives.
>    SourceManager &SM; ///< Used to read and manage source files.
> @@ -44,11 +42,13 @@ class InclusionRewriter : public PPCallb
>    const llvm::MemoryBuffer *PredefinesBuffer; ///< The preprocessor
> predefines.
>    bool ShowLineMarkers; ///< Show #line markers.
>    bool UseLineDirectives; ///< Use of line directives or line markers.
> -  typedef std::map<unsigned, FileChange> FileChangeMap;
> -  FileChangeMap FileChanges; ///< Tracks which files were included where.
> -  /// Used transitively for building up the FileChanges mapping over the
> +  /// Tracks where inclusions that change the file are found.
> +  std::map<unsigned, IncludedFile> FileIncludes;
> +  /// Tracks where inclusions that import modules are found.
> +  std::map<unsigned, const Module *> ModuleIncludes;
> +  /// Used transitively for building up the FileIncludes mapping over the
>    /// various \c PPCallbacks callbacks.
> -  FileChangeMap::iterator LastInsertedFileChange;
> +  SourceLocation LastInclusionLocation;
>  public:
>    InclusionRewriter(Preprocessor &PP, raw_ostream &OS, bool
> ShowLineMarkers,
>                      bool UseLineDirectives);
> @@ -82,7 +82,8 @@ private:
>    bool HandleHasInclude(FileID FileId, Lexer &RawLex,
>                          const DirectoryLookup *Lookup, Token &Tok,
>                          bool &FileExists);
> -  const FileChange *FindFileChangeLocation(SourceLocation Loc) const;
> +  const IncludedFile *FindIncludeAtLocation(SourceLocation Loc) const;
> +  const Module *FindModuleAtLocation(SourceLocation Loc) const;
>    StringRef NextIdentifierName(Lexer &RawLex, Token &RawToken);
>  };
>
> @@ -95,7 +96,7 @@ InclusionRewriter::InclusionRewriter(Pre
>      : PP(PP), SM(PP.getSourceManager()), OS(OS), MainEOL("\n"),
>        PredefinesBuffer(nullptr), ShowLineMarkers(ShowLineMarkers),
>        UseLineDirectives(UseLineDirectives),
> -      LastInsertedFileChange(FileChanges.end()) {}
> +      LastInclusionLocation(SourceLocation()) {}
>
>  /// Write appropriate line information as either #line directives or GNU
> line
>  /// markers depending on what mode we're in, including the \p Filename and
> @@ -143,12 +144,14 @@ void InclusionRewriter::FileChanged(Sour
>                                      FileID) {
>    if (Reason != EnterFile)
>      return;
> -  if (LastInsertedFileChange == FileChanges.end())
> +  if (LastInclusionLocation.isInvalid())
>      // we didn't reach this file (eg: the main file) via an inclusion
> directive
>      return;
> -  LastInsertedFileChange->second.Id = FullSourceLoc(Loc, SM).getFileID();
> -  LastInsertedFileChange->second.FileType = NewFileType;
> -  LastInsertedFileChange = FileChanges.end();
> +  FileID Id = FullSourceLoc(Loc, SM).getFileID();
> +  auto P = FileIncludes.emplace(LastInclusionLocation.getRawEncoding(),
> +                                IncludedFile(Id, NewFileType));
> +  assert(P.second && "Unexpected revisitation of the same include
> directive");
> +  LastInclusionLocation = SourceLocation();
>  }
>
>  /// Called whenever an inclusion is skipped due to canonical header
> protection
> @@ -156,10 +159,9 @@ void InclusionRewriter::FileChanged(Sour
>  void InclusionRewriter::FileSkipped(const FileEntry &/*SkippedFile*/,
>                                      const Token &/*FilenameTok*/,
>                                      SrcMgr::CharacteristicKind
> /*FileType*/) {
> -  assert(LastInsertedFileChange != FileChanges.end() && "A file, that
> wasn't "
> -    "found via an inclusion directive, was skipped");
> -  FileChanges.erase(LastInsertedFileChange);
> -  LastInsertedFileChange = FileChanges.end();
> +  assert(!LastInclusionLocation.isInvalid() &&
> +         "A file, that wasn't found via an inclusion directive, was
> skipped");
> +  LastInclusionLocation = SourceLocation();
>  }
>
>  /// This should be called whenever the preprocessor encounters include
> @@ -176,25 +178,36 @@ void InclusionRewriter::InclusionDirecti
>                                             StringRef /*SearchPath*/,
>                                             StringRef /*RelativePath*/,
>                                             const Module *Imported) {
> -  assert(LastInsertedFileChange == FileChanges.end() && "Another
> inclusion "
> -    "directive was found before the previous one was processed");
> -  std::pair<FileChangeMap::iterator, bool> p = FileChanges.insert(
> -    std::make_pair(HashLoc.getRawEncoding(), FileChange(HashLoc,
> Imported)));
> -  assert(p.second && "Unexpected revisitation of the same include
> directive");
> -  if (!Imported)
> -    LastInsertedFileChange = p.first;
> +  assert(LastInclusionLocation.isInvalid() &&
> +         "Another inclusion directive was found before the previous one "
> +         "was processed");
> +  if (Imported) {
> +    auto P = ModuleIncludes.emplace(HashLoc.getRawEncoding(), Imported);
> +    assert(P.second && "Unexpected revisitation of the same include
> directive");
> +  } else
> +    LastInclusionLocation = HashLoc;
>  }
>
>  /// Simple lookup for a SourceLocation (specifically one denoting the
> hash in
>  /// an inclusion directive) in the map of inclusion information,
> FileChanges.
> -const InclusionRewriter::FileChange *
> -InclusionRewriter::FindFileChangeLocation(SourceLocation Loc) const {
> -  FileChangeMap::const_iterator I =
> FileChanges.find(Loc.getRawEncoding());
> -  if (I != FileChanges.end())
> +const InclusionRewriter::IncludedFile *
> +InclusionRewriter::FindIncludeAtLocation(SourceLocation Loc) const {
> +  const auto I = FileIncludes.find(Loc.getRawEncoding());
> +  if (I != FileIncludes.end())
>      return &I->second;
>    return nullptr;
>  }
>
> +/// Simple lookup for a SourceLocation (specifically one denoting the
> hash in
> +/// an inclusion directive) in the map of module inclusion information.
> +const Module *
> +InclusionRewriter::FindModuleAtLocation(SourceLocation Loc) const {
> +  const auto I = ModuleIncludes.find(Loc.getRawEncoding());
> +  if (I != ModuleIncludes.end())
> +    return I->second;
> +  return nullptr;
> +}
> +
>  /// Detect the likely line ending style of \p FromFile by examining the
> first
>  /// newline found within it.
>  static StringRef DetectEOL(const MemoryBuffer &FromFile) {
> @@ -388,8 +401,7 @@ bool InclusionRewriter::Process(FileID F
>  {
>    bool Invalid;
>    const MemoryBuffer &FromFile = *SM.getBuffer(FileId, &Invalid);
> -  if (Invalid) // invalid inclusion
> -    return false;
> +  assert(!Invalid && "Attempting to process invalid inclusion");
>    const char *FileName = FromFile.getBufferIdentifier();
>    Lexer RawLex(FileId, &FromFile, PP.getSourceManager(),
> PP.getLangOpts());
>    RawLex.SetCommentRetentionState(false);
> @@ -433,13 +445,12 @@ bool InclusionRewriter::Process(FileID F
>              if (FileId != PP.getPredefinesFileID())
>                WriteLineInfo(FileName, Line - 1, FileType, "");
>              StringRef LineInfoExtra;
> -            if (const FileChange *Change = FindFileChangeLocation(
> -                HashToken.getLocation())) {
> -              if (Change->Mod) {
> -                WriteImplicitModuleImport(Change->Mod);
> -
> -              // else now include and recursively process the file
> -              } else if (Process(Change->Id, Change->FileType)) {
> +            SourceLocation Loc = HashToken.getLocation();
> +            if (const Module *Mod = FindModuleAtLocation(Loc))
> +              WriteImplicitModuleImport(Mod);
> +            else if (const IncludedFile *Inc =
> FindIncludeAtLocation(Loc)) {
> +              // include and recursively process the file
> +              if (Process(Inc->Id, Inc->FileType)) {
>                  // and set lineinfo back to this file, if the nested one
> was
>                  // actually included
>                  // `2' indicates returning to a file (after having
> included
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150630/7ef80615/attachment.html>


More information about the cfe-commits mailing list