On Tue, Jun 30, 2015 at 9:40 PM, Justin Bogner <m...@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@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits