<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 30, 2015 at 9:40 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: bogner<br>
Date: Tue Jun 30 23:40:10 2015<br>
New Revision: 241140<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D241140-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=NBCvW_AGTS--nFo9PpgpedTMJAJ3tORnFWixb9V8TFE&s=gU7QlL9dpwFlBfaLwmYb6lirz0d25SzDzpKdbmpuxyk&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=241140&view=rev</a><br>
Log:<br>
-frewrite-includes: Rework how includes and modules are differentiated<br>
<br>
The map of FileChange structs here was storing two disjoint types of<br>
information:<br>
<br>
1. A pointer to the Module that an #include directive implicitly<br>
imported<br>
<br>
2. A FileID and FileType for an included file. These would be left<br>
uninitialized in the Module case.<br>
<br>
This change splits these two kinds of information into their own maps,<br>
which both simplifies how we access either and avoids the undefined<br>
behaviour we were hitting due to the uninitialized fields in the<br>
included file case.<br>
<br>
Mostly NFC, but fixes some errors found by self-host with ubsan.<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp<br>
<br>
Modified: cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Frontend_Rewrite_InclusionRewriter.cpp-3Frev-3D241140-26r1-3D241139-26r2-3D241140-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=NBCvW_AGTS--nFo9PpgpedTMJAJ3tORnFWixb9V8TFE&s=GSObrLJJ2pvSKuPGxTUDMdiAoOxYAqxCggWbKFTQ270&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp?rev=241140&r1=241139&r2=241140&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp Tue Jun 30 23:40:10 2015<br>
@@ -29,13 +29,11 @@ namespace {<br>
class InclusionRewriter : public PPCallbacks {<br>
/// Information about which #includes were actually performed,<br>
/// created by preprocessor callbacks.<br>
- struct FileChange {<br>
- const Module *Mod;<br>
- SourceLocation From;<br>
+ struct IncludedFile {<br>
FileID Id;<br>
SrcMgr::CharacteristicKind FileType;<br>
- FileChange(SourceLocation From, const Module *Mod) : Mod(Mod), From(From) {<br>
- }<br>
+ IncludedFile(FileID Id, SrcMgr::CharacteristicKind FileType)<br>
+ : Id(Id), FileType(FileType) {}<br>
};<br>
Preprocessor &PP; ///< Used to find inclusion directives.<br>
SourceManager &SM; ///< Used to read and manage source files.<br>
@@ -44,11 +42,13 @@ class InclusionRewriter : public PPCallb<br>
const llvm::MemoryBuffer *PredefinesBuffer; ///< The preprocessor predefines.<br>
bool ShowLineMarkers; ///< Show #line markers.<br>
bool UseLineDirectives; ///< Use of line directives or line markers.<br>
- typedef std::map<unsigned, FileChange> FileChangeMap;<br>
- FileChangeMap FileChanges; ///< Tracks which files were included where.<br>
- /// Used transitively for building up the FileChanges mapping over the<br>
+ /// Tracks where inclusions that change the file are found.<br>
+ std::map<unsigned, IncludedFile> FileIncludes;<br>
+ /// Tracks where inclusions that import modules are found.<br>
+ std::map<unsigned, const Module *> ModuleIncludes;<br>
+ /// Used transitively for building up the FileIncludes mapping over the<br>
/// various \c PPCallbacks callbacks.<br>
- FileChangeMap::iterator LastInsertedFileChange;<br>
+ SourceLocation LastInclusionLocation;<br>
public:<br>
InclusionRewriter(Preprocessor &PP, raw_ostream &OS, bool ShowLineMarkers,<br>
bool UseLineDirectives);<br>
@@ -82,7 +82,8 @@ private:<br>
bool HandleHasInclude(FileID FileId, Lexer &RawLex,<br>
const DirectoryLookup *Lookup, Token &Tok,<br>
bool &FileExists);<br>
- const FileChange *FindFileChangeLocation(SourceLocation Loc) const;<br>
+ const IncludedFile *FindIncludeAtLocation(SourceLocation Loc) const;<br>
+ const Module *FindModuleAtLocation(SourceLocation Loc) const;<br>
StringRef NextIdentifierName(Lexer &RawLex, Token &RawToken);<br>
};<br>
<br>
@@ -95,7 +96,7 @@ InclusionRewriter::InclusionRewriter(Pre<br>
: PP(PP), SM(PP.getSourceManager()), OS(OS), MainEOL("\n"),<br>
PredefinesBuffer(nullptr), ShowLineMarkers(ShowLineMarkers),<br>
UseLineDirectives(UseLineDirectives),<br>
- LastInsertedFileChange(FileChanges.end()) {}<br>
+ LastInclusionLocation(SourceLocation()) {}<br>
<br>
/// Write appropriate line information as either #line directives or GNU line<br>
/// markers depending on what mode we're in, including the \p Filename and<br>
@@ -143,12 +144,14 @@ void InclusionRewriter::FileChanged(Sour<br>
FileID) {<br>
if (Reason != EnterFile)<br>
return;<br>
- if (LastInsertedFileChange == FileChanges.end())<br>
+ if (LastInclusionLocation.isInvalid())<br>
// we didn't reach this file (eg: the main file) via an inclusion directive<br>
return;<br>
- LastInsertedFileChange->second.Id = FullSourceLoc(Loc, SM).getFileID();<br>
- LastInsertedFileChange->second.FileType = NewFileType;<br>
- LastInsertedFileChange = FileChanges.end();<br>
+ FileID Id = FullSourceLoc(Loc, SM).getFileID();<br>
+ auto P = FileIncludes.emplace(LastInclusionLocation.getRawEncoding(),<br>
+ IncludedFile(Id, NewFileType));<br>
+ assert(P.second && "Unexpected revisitation of the same include directive");<br>
+ LastInclusionLocation = SourceLocation();<br>
}<br>
<br>
/// Called whenever an inclusion is skipped due to canonical header protection<br>
@@ -156,10 +159,9 @@ void InclusionRewriter::FileChanged(Sour<br>
void InclusionRewriter::FileSkipped(const FileEntry &/*SkippedFile*/,<br>
const Token &/*FilenameTok*/,<br>
SrcMgr::CharacteristicKind /*FileType*/) {<br>
- assert(LastInsertedFileChange != FileChanges.end() && "A file, that wasn't "<br>
- "found via an inclusion directive, was skipped");<br>
- FileChanges.erase(LastInsertedFileChange);<br>
- LastInsertedFileChange = FileChanges.end();<br>
+ assert(!LastInclusionLocation.isInvalid() &&<br>
+ "A file, that wasn't found via an inclusion directive, was skipped");<br>
+ LastInclusionLocation = SourceLocation();<br>
}<br>
<br>
/// This should be called whenever the preprocessor encounters include<br>
@@ -176,25 +178,36 @@ void InclusionRewriter::InclusionDirecti<br>
StringRef /*SearchPath*/,<br>
StringRef /*RelativePath*/,<br>
const Module *Imported) {<br>
- assert(LastInsertedFileChange == FileChanges.end() && "Another inclusion "<br>
- "directive was found before the previous one was processed");<br>
- std::pair<FileChangeMap::iterator, bool> p = FileChanges.insert(<br>
- std::make_pair(HashLoc.getRawEncoding(), FileChange(HashLoc, Imported)));<br>
- assert(p.second && "Unexpected revisitation of the same include directive");<br>
- if (!Imported)<br>
- LastInsertedFileChange = p.first;<br>
+ assert(LastInclusionLocation.isInvalid() &&<br>
+ "Another inclusion directive was found before the previous one "<br>
+ "was processed");<br>
+ if (Imported) {<br>
+ auto P = ModuleIncludes.emplace(HashLoc.getRawEncoding(), Imported);<br>
+ assert(P.second && "Unexpected revisitation of the same include directive");<br>
+ } else<br>
+ LastInclusionLocation = HashLoc;<br>
}<br>
<br>
/// Simple lookup for a SourceLocation (specifically one denoting the hash in<br>
/// an inclusion directive) in the map of inclusion information, FileChanges.<br>
-const InclusionRewriter::FileChange *<br>
-InclusionRewriter::FindFileChangeLocation(SourceLocation Loc) const {<br>
- FileChangeMap::const_iterator I = FileChanges.find(Loc.getRawEncoding());<br>
- if (I != FileChanges.end())<br>
+const InclusionRewriter::IncludedFile *<br>
+InclusionRewriter::FindIncludeAtLocation(SourceLocation Loc) const {<br>
+ const auto I = FileIncludes.find(Loc.getRawEncoding());<br>
+ if (I != FileIncludes.end())<br>
return &I->second;<br>
return nullptr;<br>
}<br>
<br>
+/// Simple lookup for a SourceLocation (specifically one denoting the hash in<br>
+/// an inclusion directive) in the map of module inclusion information.<br>
+const Module *<br>
+InclusionRewriter::FindModuleAtLocation(SourceLocation Loc) const {<br>
+ const auto I = ModuleIncludes.find(Loc.getRawEncoding());<br>
+ if (I != ModuleIncludes.end())<br>
+ return I->second;<br>
+ return nullptr;<br>
+}<br>
+<br>
/// Detect the likely line ending style of \p FromFile by examining the first<br>
/// newline found within it.<br>
static StringRef DetectEOL(const MemoryBuffer &FromFile) {<br>
@@ -388,8 +401,7 @@ bool InclusionRewriter::Process(FileID F<br>
{<br>
bool Invalid;<br>
const MemoryBuffer &FromFile = *SM.getBuffer(FileId, &Invalid);<br>
- if (Invalid) // invalid inclusion<br>
- return false;<br>
+ assert(!Invalid && "Attempting to process invalid inclusion");<br>
const char *FileName = FromFile.getBufferIdentifier();<br>
Lexer RawLex(FileId, &FromFile, PP.getSourceManager(), PP.getLangOpts());<br>
RawLex.SetCommentRetentionState(false);<br>
@@ -433,13 +445,12 @@ bool InclusionRewriter::Process(FileID F<br>
if (FileId != PP.getPredefinesFileID())<br>
WriteLineInfo(FileName, Line - 1, FileType, "");<br>
StringRef LineInfoExtra;<br>
- if (const FileChange *Change = FindFileChangeLocation(<br>
- HashToken.getLocation())) {<br>
- if (Change->Mod) {<br>
- WriteImplicitModuleImport(Change->Mod);<br>
-<br>
- // else now include and recursively process the file<br>
- } else if (Process(Change->Id, Change->FileType)) {<br>
+ SourceLocation Loc = HashToken.getLocation();<br>
+ if (const Module *Mod = FindModuleAtLocation(Loc))<br>
+ WriteImplicitModuleImport(Mod);<br>
+ else if (const IncludedFile *Inc = FindIncludeAtLocation(Loc)) {<br>
+ // include and recursively process the file<br>
+ if (Process(Inc->Id, Inc->FileType)) {<br>
// and set lineinfo back to this file, if the nested one was<br>
// actually included<br>
// `2' indicates returning to a file (after having included<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>