llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (Evianaive) <details> <summary>Changes</summary> This patch implements support for undo to properly revert file inclusion (#include) and macro definitions in clang interpreter. --- Full diff: https://github.com/llvm/llvm-project/pull/182750.diff 12 Files Affected: - (modified) clang/include/clang/Basic/FileManager.h (+9) - (modified) clang/include/clang/Basic/SourceManager.h (+3-1) - (modified) clang/include/clang/Interpreter/PartialTranslationUnit.h (+33) - (modified) clang/include/clang/Lex/Preprocessor.h (+5) - (modified) clang/lib/Basic/FileManager.cpp (+38-3) - (modified) clang/lib/Basic/SourceManager.cpp (+11) - (modified) clang/lib/Interpreter/IncrementalParser.cpp (+96-3) - (modified) clang/lib/Interpreter/IncrementalParser.h (+6-1) - (modified) clang/lib/Interpreter/Interpreter.cpp (+2-2) - (modified) clang/lib/Lex/PPMacroExpansion.cpp (+14) - (added) clang/test/Interpreter/Inputs/dynamic-header.h (+8) - (modified) clang/test/Interpreter/code-undo.cpp (+24) ``````````diff diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index fa7552b6ae41d..3e8f249e5d59c 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -32,6 +32,7 @@ #include <map> #include <memory> #include <string> +#include <set> namespace llvm { @@ -106,6 +107,11 @@ class FileManager : public RefCountedBase<FileManager> { /// The canonical names of files and directories . llvm::DenseMap<const void *, llvm::StringRef> CanonicalNames; + + std::set<const FileEntry*> FileEntriesToReread; + + /// The canonical names of directories. + llvm::DenseMap<const DirectoryEntry *, llvm::StringRef> CanonicalDirNames; /// Storage for canonical names that we have computed. llvm::BumpPtrAllocator CanonicalNameStorage; @@ -284,6 +290,9 @@ class FileManager : public RefCountedBase<FileManager> { std::error_code getNoncachedStatValue(StringRef Path, llvm::vfs::Status &Result); + /// Remove the real file \p Entry from the cache. + void invalidateCache(FileEntryRef Entry); + /// If path is not absolute and FileSystemOptions set the working /// directory, the path is modified to be relative to the given /// working directory. diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index bc9e97863556d..334854c5ba798 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -256,7 +256,7 @@ class alignas(8) ContentCache { /// Set the buffer. void setBuffer(std::unique_ptr<llvm::MemoryBuffer> B) { - IsBufferInvalid = false; + IsBufferInvalid = !B; Buffer = std::move(B); } @@ -849,6 +849,8 @@ class SourceManager : public RefCountedBase<SourceManager> { void clearIDTables(); + void invalidateCache(FileID FID); + /// Initialize this source manager suitably to replay the compilation /// described by \p Old. Requires that \p Old outlive \p *this. void initializeForReplay(const SourceManager &Old); diff --git a/clang/include/clang/Interpreter/PartialTranslationUnit.h b/clang/include/clang/Interpreter/PartialTranslationUnit.h index c878e139fe70d..86031ace8ba86 100644 --- a/clang/include/clang/Interpreter/PartialTranslationUnit.h +++ b/clang/include/clang/Interpreter/PartialTranslationUnit.h @@ -34,6 +34,39 @@ struct PartialTranslationUnit { bool operator==(const PartialTranslationUnit &other) { return other.TUPart == TUPart && other.TheModule == TheModule; } + + ///\brief Each macro pair (is this the same as for decls?)came + /// through different interface at + /// different time. We are being conservative and we want to keep all the + /// call sequence that originally occurred in clang. + /// + struct MacroDirectiveInfo { + // We need to store both the IdentifierInfo and the MacroDirective + // because the Preprocessor stores the macros in a DenseMap<II, MD>. + IdentifierInfo* II; + const MacroDirective* MD; + MacroDirectiveInfo(IdentifierInfo* II, + const MacroDirective* MD) + : II(II), MD(MD) {} + inline bool operator==(const MacroDirectiveInfo& rhs) const { + return II == rhs.II && MD == rhs.MD; + } + inline bool operator!=(const MacroDirectiveInfo& rhs) const { + return !operator==(rhs); + } + }; + // Intentionally use struct instead of pair because we don't need default + // init. + // Add macro decls to be able to revert them for error recovery. + typedef llvm::SmallVector<MacroDirectiveInfo, 2> MacroDirectiveInfoQueue; + + ///\brief All seen macros. + /// + MacroDirectiveInfoQueue MacroDirectiveInfos; + + /// Files that were #included during this PTU's parsing. + /// Used to reset HeaderFileInfo on Undo. + std::vector<FileEntryRef> IncludedFiles; }; } // namespace clang diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index b6e42a6151ac3..4b43ba810ac97 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -1493,6 +1493,11 @@ class Preprocessor { return appendDefMacroDirective(II, MI, MI->getDefinitionLoc()); } + /// Remove a IdentifierInfo and MacroDirective from the history. + /// Given an IdentifierInfo and a MacroDirective we can remove them from + /// the macros vector. + void removeMacro(IdentifierInfo *II, MacroDirective *MD); + /// Set a MacroDirective that was loaded from a PCH file. void setLoadedMacroDirective(IdentifierInfo *II, MacroDirective *ED, MacroDirective *MD); diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index 44092efd74717..ebc02d0bff50f 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -217,7 +217,25 @@ llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename, // See if there is already an entry in the map. auto SeenFileInsertResult = SeenFileEntries.insert({Filename, std::errc::no_such_file_or_directory}); - if (!SeenFileInsertResult.second) { + + auto *NamedFileEnt = &*SeenFileInsertResult.first; + + const FileEntry *StaleFileEntry = 0; + bool needsRereading = false; + if (NamedFileEnt && NamedFileEnt->getValue()) { + FileEntryRef::MapValue Value = *NamedFileEnt->getValue(); + if (Value.V.is<FileEntry *>()) { + auto found = FileEntriesToReread.find(Value.V.get<FileEntry*>()); + if (found != FileEntriesToReread.end()) { + needsRereading = true; + StaleFileEntry = *found; + FileEntriesToReread.erase(found); + } + } + } + + // See if there is already an entry in the map. + if (!SeenFileInsertResult.second && !needsRereading) { if (!SeenFileInsertResult.first->second) return llvm::errorCodeToError( SeenFileInsertResult.first->second.getError()); @@ -226,8 +244,6 @@ llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename, // We've not seen this before. Fill it in. ++NumFileCacheMisses; - auto *NamedFileEnt = &*SeenFileInsertResult.first; - assert(!NamedFileEnt->second && "should be newly-created"); // Get the null-terminated file name as stored as the key of the // SeenFileEntries map. @@ -353,6 +369,20 @@ llvm::Expected<FileEntryRef> FileManager::getFileRef(StringRef Filename, // We should still fill the path even if we aren't opening the file. fillRealPathName(UFE, InterndFileName); } + + if (StaleFileEntry) { + // Find occurrences of old FileEntry; update with new one: + for (auto& fe: SeenFileEntries) { + if (fe.getValue()) { + FileEntryRef::MapValue Value = *fe.getValue(); + if (Value.V.is<FileEntry *>() + && Value.V.get<FileEntry*>() == StaleFileEntry) { + fe.setValue(FileEntryRef::MapValue(*UFE, DirInfo)); + } + } + } + } + return ReturnedRef; } @@ -610,6 +640,11 @@ FileManager::getNoncachedStatValue(StringRef Path, return std::error_code(); } +void FileManager::invalidateCache(FileEntryRef Entry) { + assert(Entry && "Cannot invalidate a NULL FileEntry"); + FileEntriesToReread.insert(Entry); +} + void FileManager::GetUniqueIDMapping( SmallVectorImpl<OptionalFileEntryRef> &UIDToFiles) const { UIDToFiles.clear(); diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index b6cc6ec9365f5..2ec21f6157b82 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -358,6 +358,17 @@ bool SourceManager::isMainFile(const FileEntry &SourceFile) { return false; } +void SourceManager::invalidateCache(FileID FID) { + OptionalFileEntryRef Entry = getFileEntryRefForID(FID); + if (!Entry) + return; + if (ContentCache *&E = FileInfos[*Entry]) { + E->setBuffer(nullptr); + E = 0; + } + getFileManager().invalidateCache(*Entry); +} + void SourceManager::initializeForReplay(const SourceManager &Old) { assert(MainFileID.isInvalid() && "expected uninitialized SourceManager"); diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp index bf08911e23533..44101944eb8b6 100644 --- a/clang/lib/Interpreter/IncrementalParser.cpp +++ b/clang/lib/Interpreter/IncrementalParser.cpp @@ -16,6 +16,7 @@ #include "clang/AST/DeclContextInternals.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Interpreter/PartialTranslationUnit.h" +#include "clang/Lex/PPCallbacks.h" #include "clang/Parse/Parser.h" #include "clang/Sema/Sema.h" #include "llvm/IR/Module.h" @@ -28,6 +29,66 @@ namespace clang { +class IncrementalPreProcessorTracker : public PPCallbacks { + std::vector<FileEntryRef> IncludedFiles; + PartialTranslationUnit::MacroDirectiveInfoQueue MacroDirectives; + void MacroDirective(const clang::Token& MacroNameTok, + const clang::MacroDirective* MD) { + PartialTranslationUnit::MacroDirectiveInfo MDE(MacroNameTok.getIdentifierInfo(), MD); + MacroDirectives.push_back(MDE); + } +public: + explicit IncrementalPreProcessorTracker() {} + void InclusionDirective( + SourceLocation HashLoc, + const Token &IncludeTok, + StringRef FileName, + bool IsAngled, + CharSourceRange FilenameRange, + OptionalFileEntryRef File, + StringRef SearchPath, + StringRef RelativePath, + const Module *SuggestedModule, + bool ModuleImported, + SrcMgr::CharacteristicKind FileType) override { + + if (File) { + IncludedFiles.push_back(*File); + } + } + + /// \name PPCallbacks overrides + /// Macro support + void MacroDefined(const clang::Token& MacroNameTok, + const clang::MacroDirective* MD) final { + MacroDirective(MacroNameTok, MD); + } + + /// \name PPCallbacks overrides + /// Macro support + void MacroUndefined(const clang::Token& MacroNameTok, + const clang::MacroDefinition& /*MD*/, + const clang::MacroDirective* Undef) final { + if (Undef) + MacroDirective(MacroNameTok, Undef); + } + + /// Move out tracked files after Parse completes + std::vector<FileEntryRef> takeFiles() { + return std::move(IncludedFiles); + } + + /// Move out tracked macros after Parse completes + PartialTranslationUnit::MacroDirectiveInfoQueue takeMacros() { + return std::move(MacroDirectives); + } + + void reset() { + IncludedFiles.clear(); + MacroDirectives.clear(); + } +}; + // IncrementalParser::IncrementalParser() {} IncrementalParser::IncrementalParser(CompilerInstance &Instance, @@ -42,6 +103,11 @@ IncrementalParser::IncrementalParser(CompilerInstance &Instance, External->StartTranslationUnit(Consumer); P->Initialize(); + + // track files that are included when parse to support undo + auto Tracker = std::make_unique<IncrementalPreProcessorTracker>(); + PreProcessorTracker = Tracker.get(); + S.getPreprocessor().addPPCallbacks(std::move(Tracker)); } IncrementalParser::~IncrementalParser() { P.reset(); } @@ -82,7 +148,7 @@ IncrementalParser::ParseOrWrapTopLevelDecl() { DiagnosticsEngine &Diags = S.getDiagnostics(); if (Diags.hasErrorOccurred()) { - CleanUpPTU(C.getTranslationUnitDecl()); + CleanUpTU(C.getTranslationUnitDecl()); Diags.Reset(/*soft=*/true); Diags.getClient()->clear(); @@ -108,7 +174,10 @@ llvm::Expected<TranslationUnitDecl *> IncrementalParser::Parse(llvm::StringRef input) { Preprocessor &PP = S.getPreprocessor(); assert(PP.isIncrementalProcessingEnabled() && "Not in incremental mode!?"); - + + if (PreProcessorTracker) + PreProcessorTracker->reset(); + std::ostringstream SourceName; SourceName << "input_line_" << InputCount++; @@ -161,7 +230,7 @@ IncrementalParser::Parse(llvm::StringRef input) { return PTU; } -void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) { +void IncrementalParser::CleanUpTU(TranslationUnitDecl *MostRecentTU) { if (StoredDeclsMap *Map = MostRecentTU->getPrimaryContext()->getLookupPtr()) { for (auto &&[Key, List] : *Map) { DeclContextLookupResult R = List.getLookupResult(); @@ -194,6 +263,25 @@ void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) { } } +void IncrementalParser::CleanUpPTU(const PartialTranslationUnit &MostRecentPTU) { + CleanUpTU(MostRecentPTU.TUPart); + + auto& PP = P->getPreprocessor(); + auto& HS = PP.getHeaderSearchInfo(); + auto& FM = PP.getFileManager(); + if (!PTUs.back().IncludedFiles.empty()) { + for (FileEntryRef FE : PTUs.back().IncludedFiles) { + HeaderFileInfo& HFI = HS.getFileInfo(FE); + HFI.IsLocallyIncluded = false; + HFI.isPragmaOnce = false; + HFI.LazyControllingMacro = LazyIdentifierInfoPtr(); + HFI.IsValid = false; + + FM.invalidateCache(FE); + } + } +} + PartialTranslationUnit & IncrementalParser::RegisterPTU(TranslationUnitDecl *TU, std::unique_ptr<llvm::Module> M /*={}*/) { @@ -213,6 +301,11 @@ IncrementalParser::RegisterPTU(TranslationUnitDecl *TU, LLVM_DEBUG(llvm::dbgs() << ", M=" << LastPTU.TheModule.get() << " (" << LastPTU.TheModule->getName() << ")"); LLVM_DEBUG(llvm::dbgs() << "]\n"); + + if (PreProcessorTracker) { + LastPTU.IncludedFiles = std::move(PreProcessorTracker->takeFiles()); + LastPTU.MacroDirectiveInfos = std::move(PreProcessorTracker->takeMacros()); + } return LastPTU; } } // end namespace clang diff --git a/clang/lib/Interpreter/IncrementalParser.h b/clang/lib/Interpreter/IncrementalParser.h index 9b042bc494efb..743dfbf8679e4 100644 --- a/clang/lib/Interpreter/IncrementalParser.h +++ b/clang/lib/Interpreter/IncrementalParser.h @@ -30,6 +30,7 @@ class Parser; class Sema; class TranslationUnitDecl; class IncrementalAction; +class IncrementalPreProcessorTracker; struct PartialTranslationUnit; /// Provides support for incremental compilation. Keeps track of the state @@ -53,6 +54,9 @@ class IncrementalParser { IncrementalAction *Act = nullptr; std::list<PartialTranslationUnit> &PTUs; + + /// Tracks the include files during parsing + IncrementalPreProcessorTracker* PreProcessorTracker; public: IncrementalParser(CompilerInstance &Instance, IncrementalAction *Act, @@ -64,8 +68,9 @@ class IncrementalParser { /// \c TranslationUnitDecl. virtual llvm::Expected<TranslationUnitDecl *> Parse(llvm::StringRef Input); - void CleanUpPTU(TranslationUnitDecl *MostRecentTU); + void CleanUpTU(TranslationUnitDecl *MostRecentTU); + void CleanUpPTU(const PartialTranslationUnit& MostRecentPTU); /// Register a PTU produced by Parse. PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU, std::unique_ptr<llvm::Module> M = {}); diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp index 9c94cfa5ee381..327f65529b767 100644 --- a/clang/lib/Interpreter/Interpreter.cpp +++ b/clang/lib/Interpreter/Interpreter.cpp @@ -585,14 +585,14 @@ llvm::Error Interpreter::Undo(unsigned N) { getEffectivePTUSize()), std::error_code()); } - + for (unsigned I = 0; I < N; I++) { if (IncrExecutor) { if (llvm::Error Err = IncrExecutor->removeModule(PTUs.back())) return Err; } - IncrParser->CleanUpPTU(PTUs.back().TUPart); + IncrParser->CleanUpPTU(PTUs.back()); PTUs.pop_back(); } return llvm::Error::success(); diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index 20b8fe585a007..36e346539b161 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -91,6 +91,20 @@ void Preprocessor::appendMacroDirective(IdentifierInfo *II, MacroDirective *MD){ II->setChangedSinceDeserialization(); } +void Preprocessor::removeMacro(IdentifierInfo *II, MacroDirective *MD) { + assert(II && MD); + II->setHasMacroDefinition(false); + CurSubmoduleState->Macros.erase(II); + if (MacroDirective* prevMD = MD->getPrevious()) { + // Avoid assertion in appendMacroDirective. + MacroDirective* prevPrevMD = prevMD->getPrevious(); + prevMD->setPrevious(0); + appendMacroDirective(II, prevMD); + prevMD->setPrevious(prevPrevMD); + } +} + + void Preprocessor::setLoadedMacroDirective(IdentifierInfo *II, MacroDirective *ED, MacroDirective *MD) { diff --git a/clang/test/Interpreter/Inputs/dynamic-header.h b/clang/test/Interpreter/Inputs/dynamic-header.h new file mode 100644 index 0000000000000..b5962083ba4bb --- /dev/null +++ b/clang/test/Interpreter/Inputs/dynamic-header.h @@ -0,0 +1,8 @@ +#ifndef DYNAMIC_HEADER_H +#define DYNAMIC_HEADER_H + +inline int getDynamicValue() { + return 100; +} + +#endif diff --git a/clang/test/Interpreter/code-undo.cpp b/clang/test/Interpreter/code-undo.cpp index 4516910ca3b4f..ca977261e3e3c 100644 --- a/clang/test/Interpreter/code-undo.cpp +++ b/clang/test/Interpreter/code-undo.cpp @@ -20,4 +20,28 @@ auto r4 = bar(); %undo auto r5 = bar(); +//--- Test file re-inclusion after undo with in-repl modification --- +// RUN: rm -rf %T && mkdir -p %T +// RUN: cp %S/Inputs/dynamic-header.h %T/dynamic-header-test.h +// RUN: cat %s | clang-repl -I%T | FileCheck %s +#include <cstdio> + +#include "dynamic-header-test.h" +auto val1 = getDynamicValue(); +%undo +%undo +// CHECK: val1 = 100 +{ + FILE *f; + fopen_s(&f, "%T/dynamic-header-test.h", "w"); + fprintf(f, "#ifndef DYNAMIC_HEADER_H\n"); + fprintf(f, "#define DYNAMIC_HEADER_H\n"); + fprintf(f, "inline int getDynamicValue() { return 200; }\n"); + fprintf(f, "#endif\n"); + fclose(f); +} +#include "dynamic-header-test.h" +auto val2 = getDynamicValue(); +// CHECK-NEXT: val2 = 200 + %quit `````````` </details> https://github.com/llvm/llvm-project/pull/182750 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
