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