https://github.com/Vipul-Cariappa updated https://github.com/llvm/llvm-project/pull/218149
>From a3386428ad025c57212b4c3c7fc0cf96372dc0bc Mon Sep 17 00:00:00 2001 From: Vipul Cariappa <[email protected]> Date: Fri, 21 Aug 2026 23:56:37 +0530 Subject: [PATCH 1/2] [clang-repl] Keep earlier declarations alive when an input fails Fixes llvm/llvm-project#201844. --- clang/include/clang/AST/DeclCXX.h | 1 + clang/lib/Interpreter/IncrementalAction.cpp | 2 + clang/lib/Interpreter/IncrementalAction.h | 4 + clang/lib/Interpreter/IncrementalParser.cpp | 118 +++++++++++++++++- clang/lib/Interpreter/IncrementalParser.h | 9 ++ .../failed-input-keeps-redecls.cpp | 101 +++++++++++++++ 6 files changed, 232 insertions(+), 3 deletions(-) create mode 100644 clang/test/Interpreter/failed-input-keeps-redecls.cpp diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index a42884be71d68..11d3db5d99293 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -264,6 +264,7 @@ class CXXRecordDecl : public RecordDecl { friend class ASTRecordWriter; friend class ASTWriter; friend class DeclContext; + friend class IncrementalParser; friend class LambdaExpr; friend class ODRDiagsEmitter; diff --git a/clang/lib/Interpreter/IncrementalAction.cpp b/clang/lib/Interpreter/IncrementalAction.cpp index d22031c8fa893..10ca3d3a753c5 100644 --- a/clang/lib/Interpreter/IncrementalAction.cpp +++ b/clang/lib/Interpreter/IncrementalAction.cpp @@ -120,6 +120,8 @@ std::unique_ptr<llvm::Module> IncrementalAction::GenModule() { return nullptr; } +void IncrementalAction::discardModule() { GenModule(); } + CodeGenerator *IncrementalAction::getCodeGen() const { FrontendAction *WrappedAct = getWrapped(); if (!WrappedAct || !WrappedAct->hasIRSupport()) diff --git a/clang/lib/Interpreter/IncrementalAction.h b/clang/lib/Interpreter/IncrementalAction.h index 725cdd0c27cf4..31547cd785967 100644 --- a/clang/lib/Interpreter/IncrementalAction.h +++ b/clang/lib/Interpreter/IncrementalAction.h @@ -74,6 +74,10 @@ class IncrementalAction : public WrapperFrontendAction { /// Generate an LLVM module for the most recent parsed input. std::unique_ptr<llvm::Module> GenModule(); + + /// Throw away what CodeGen emitted for an input that failed to parse and + /// start a fresh module. + void discardModule(); }; class InProcessPrintingASTConsumer final : public MultiplexConsumer { diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp index 59018907056b8..a30c720873abb 100644 --- a/clang/lib/Interpreter/IncrementalParser.cpp +++ b/clang/lib/Interpreter/IncrementalParser.cpp @@ -15,7 +15,9 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/DeclContextInternals.h" +#include "clang/AST/DeclTemplate.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Interpreter/PartialTranslationUnit.h" #include "clang/Parse/Parser.h" @@ -87,6 +89,12 @@ IncrementalParser::ParseOrWrapTopLevelDecl() { if (Diags.hasErrorOccurred()) { CleanUpPTU(C.getTranslationUnitDecl()); + // Whatever got through before the error was already handed to CodeGen. It + // belongs to no PTU and will never be executed, but it stays in the module + // the next input emits into and collides with the definitions that input + // provides. Drop it and start a fresh module. + Act->discardModule(); + Diags.Reset(/*soft=*/true); Diags.getClient()->clear(); return llvm::make_error<llvm::StringError>("Parsing failed.", @@ -191,30 +199,134 @@ void IncrementalParser::withdrawMostRecentTU( C.TUDecl = Prev; } +/// The newest declaration of whatever D redeclares that still lives outside +/// DiscardedTU +static NamedDecl *findSurvivingPrevDecl(NamedDecl *D, + TranslationUnitDecl *DiscardedTU) { + for (Decl *Prev = D->getPreviousDecl(); Prev; Prev = Prev->getPreviousDecl()) + if (Prev->getTranslationUnitDecl() != DiscardedTU) + return dyn_cast<NamedDecl>(Prev); + return nullptr; +} + +/// Unlink everything a discarded re-opening of a namespace put into it. Members +/// are made visible in the namespace's primary context, which outlives the +/// discarded PTU, so leaving them behind would keep half-parsed declarations +/// reachable +static void dropContainingMembers(NamespaceDecl *ND) { + llvm::SmallVector<Decl *, 8> Members(ND->decls()); + for (Decl *M : Members) + ND->removeDecl(M); +} + +bool IncrementalParser::withdrawRedecl(NamedDecl *D, NamedDecl *Prev, + TranslationUnitDecl *DiscardedTU) { + ASTContext &C = S.getASTContext(); + + auto Unlink = [&C](auto *Latest, NamedDecl *SurvivorND) { + using T = std::remove_pointer_t<decltype(Latest)>; + auto *Survivor = cast<T>(SurvivorND); + + // Rebuild First -> ... -> Survivor -> ... -> Latest as + // First -> ... -> Survivor. + Latest->getFirstDecl()->RedeclLink.setLatest(Survivor); + + // The chain is circular: a withdrawn declaration still linked into it can + // never walk back around to itself, so redecls() on one would not + // terminate. Give each a chain of its own. + for (T *Dead = Latest; Dead != Survivor;) { + T *Next = Dead->getPreviousDecl(); + Dead->First = Dead; + Dead->RedeclLink = Redeclarable<T>::LatestDeclLink(C); + Dead = Next; + } + }; + + if (auto *TD = dyn_cast<TagDecl>(D)) { + Unlink(TD, Prev); + // A class keeps its definition outside the redeclaration chain, in the + // DefinitionData that startDefinition() hands to every redeclaration. + // Unlinking leaves the survivors pointing at the discarded definition, so + // drop it; a later definition allocates a fresh one for the whole chain. + if (auto *RD = dyn_cast<CXXRecordDecl>(Prev)) + if (CXXRecordDecl *Def = RD->getDefinition(); + Def && Def->getTranslationUnitDecl() == DiscardedTU) + for (auto *R : RD->redecls()) + cast<CXXRecordDecl>(R)->DefinitionData = nullptr; + return true; + } + if (auto *FD = dyn_cast<FunctionDecl>(D)) { + Unlink(FD, Prev); + return true; + } + if (auto *VD = dyn_cast<VarDecl>(D)) { + Unlink(VD, Prev); + return true; + } + if (auto *ND = dyn_cast<NamespaceDecl>(D)) { + dropContainingMembers(ND); + Unlink(ND, Prev); + return true; + } + if (auto *TND = dyn_cast<TypedefNameDecl>(D)) { + Unlink(TND, Prev); + return true; + } + if (auto *RTD = dyn_cast<RedeclarableTemplateDecl>(D)) { + auto *PrevRTD = cast<RedeclarableTemplateDecl>(Prev); + Unlink(RTD, Prev); + // A template declares keeps a redeclaration chain of its own, + // running alongside the template's. + return withdrawRedecl(RTD->getTemplatedDecl(), PrevRTD->getTemplatedDecl(), + DiscardedTU); + } + return false; +} + void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) { if (StoredDeclsMap *Map = MostRecentTU->getPrimaryContext()->getLookupPtr()) { // Collect the keys to erase: erasing during iteration invalidates the map // iterator under backward-shift deletion. llvm::SmallVector<DeclarationName, 16> KeysToErase; + // Declarations an earlier input made and this one only redeclared: the + // name goes back to what it meant before rather than disappearing. + llvm::SmallVector<std::pair<DeclarationName, NamedDecl *>, 4> + DeclsToRestore; for (auto &&[Key, List] : *Map) { DeclContextLookupResult R = List.getLookupResult(); std::vector<NamedDecl *> NamedDeclsToRemove; bool RemoveAll = true; for (NamedDecl *D : R) { - if (D->getTranslationUnitDecl() == MostRecentTU) - NamedDeclsToRemove.push_back(D); - else + if (D->getTranslationUnitDecl() != MostRecentTU) { RemoveAll = false; + continue; + } + NamedDeclsToRemove.push_back(D); } + // Dropping the lookup entries is not enough: what survives still + // redeclares the discarded declarations and would reach them by walking + // the chain. + llvm::SmallVector<NamedDecl *, 4> Survivors; + for (NamedDecl *D : NamedDeclsToRemove) + if (NamedDecl *Prev = findSurvivingPrevDecl(D, MostRecentTU)) + if (withdrawRedecl(D, Prev, MostRecentTU)) + Survivors.push_back(Prev); + if (LLVM_LIKELY(RemoveAll)) { KeysToErase.push_back(Key); + for (NamedDecl *Prev : Survivors) + DeclsToRestore.emplace_back(Key, Prev); } else { + // Other declarations of the name remain visible, so there is nothing + // to put back. for (NamedDecl *D : NamedDeclsToRemove) List.remove(D); } } for (DeclarationName Key : KeysToErase) Map->erase(Key); + for (auto &[Key, Prev] : DeclsToRestore) + (*Map)[Key].addOrReplaceDecl(Prev); } ExternCContextDecl *ECCD = S.getASTContext().getExternCContextDecl(); diff --git a/clang/lib/Interpreter/IncrementalParser.h b/clang/lib/Interpreter/IncrementalParser.h index b626cebaafcd7..a07ab784f4a59 100644 --- a/clang/lib/Interpreter/IncrementalParser.h +++ b/clang/lib/Interpreter/IncrementalParser.h @@ -25,6 +25,7 @@ class ASTConsumer; class CompilerInstance; class Parser; class Sema; +class NamedDecl; class TranslationUnitDecl; class IncrementalAction; struct PartialTranslationUnit; @@ -73,6 +74,14 @@ class IncrementalParser { /// Rebuild the translation unit redeclaration chain without \p MostRecentTU, /// making its predecessor the current unit again. void withdrawMostRecentTU(TranslationUnitDecl *MostRecentTU); + + /// Rebuild \p D's redeclaration chain without the + /// declarations \p DiscardedTU contributed, making \p Prev current again. + /// \returns false if \p D is of a kind we cannot unlink, in which case the + /// caller must drop the name rather than leave \p Prev pointing into + /// \p DiscardedTU. + bool withdrawRedecl(NamedDecl *D, NamedDecl *Prev, + TranslationUnitDecl *DiscardedTU); }; } // end namespace clang diff --git a/clang/test/Interpreter/failed-input-keeps-redecls.cpp b/clang/test/Interpreter/failed-input-keeps-redecls.cpp new file mode 100644 index 0000000000000..0fec65683584f --- /dev/null +++ b/clang/test/Interpreter/failed-input-keeps-redecls.cpp @@ -0,0 +1,101 @@ +// REQUIRES: host-supports-jit +// RUN: cat %s | clang-repl 2>&1 | FileCheck %s +// RUN: cat %s | clang-repl 2>&1 | FileCheck %s --check-prefix=NEG + +// A failed input must not take earlier declarations down with it, and must not +// leave anything of its own behind for a later input to trip over. + +extern "C" int printf(const char *, ...); + +namespace N { struct S { int v; }; void foo() { printf("foo\n"); } } + +namespace N { void bar() { printf("bar\n" } } +// CHECK-DAG: error: expected ')' + +// Everything N held before the failed input is still reachable. +N::foo(); +// CHECK-DAG: foo +N::S s; s.v = 7; printf("s.v = %d\n", s.v); +// CHECK-DAG: s.v = 7 + +// N is still open for business, and bar is free to be defined properly. +namespace N { void bar() { printf("bar\n"); } } +N::bar(); +// CHECK-DAG: bar +// NEG-NOT: error: call to 'bar' is ambiguous + +namespace N { void baz() { printf("baz\n"); } } +N::baz(); +// CHECK-DAG: baz + +// A name that only ever existed in a failed input stays gone. +namespace M { int m = undeclared_thing; } +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +int probe = M::m; +// CHECK-DAG: error: use of undeclared identifier 'M' + +// A class survives a failed redefinition, and the failed definition does not +// become the one everybody sees. +struct T; +struct T { int a; }; int e1 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +T *tp = nullptr; printf("T reachable %d\n", tp == nullptr); +// CHECK-DAG: T reachable 1 +struct T { int a; int b; }; +printf("sizeof(T) = %d\n", (int)sizeof(T)); +// CHECK-DAG: sizeof(T) = + +enum E : int; +enum E : int { A = 1 }; int e2 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +enum E : int { A = 1, B = 2 }; +printf("B = %d\n", (int)B); +// CHECK-DAG: B = 2 + +// The body a failed input got as far as emitting must not be what gets called. +void f(); +void f() { printf("f discarded\n"); } int e3 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +void f() { printf("f kept\n"); } +f(); +// CHECK-DAG: f kept +// NEG-NOT: {{^}}f discarded + +// Same for one overload of a set whose other overloads survive. +void g(int) { printf("g int\n"); } +void g(double) { printf("g discarded\n"); } int e4 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +g(1); +// CHECK-DAG: g int +void g(double) { printf("g kept\n"); } +g(1.5); +// CHECK-DAG: g kept +// NEG-NOT: {{^}}g discarded +g(1); +// CHECK-DAG: g int + +// A template's pattern has a redeclaration chain of its own. +template <class X> struct Box; +template <class X> struct Box { X v; }; int e5 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +template <class X> struct Box { X v; int tag; }; +Box<int> b; b.v = 5; b.tag = 6; printf("box %d %d\n", b.v, b.tag); +// CHECK-DAG: box 5 6 + +template <class X> X twice(X x, int y) { return x * y; } +template <class X> X twice(X x) { return x + x; } int e6 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +printf("twice = %d\n", twice(21, 2)); +// CHECK-DAG: twice = 42 +template <class X> X twice(X x) { return x * 2; } +printf("twice = %d\n", twice(21)); +// CHECK-DAG: twice = 42 + +extern int gv; +int gv = 1; int e7 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +int gv = 9; +printf("gv = %d\n", gv); +// CHECK-DAG: gv = 9 + +%quit >From cd0e93de398db939cb0b011df3a47a7004160930 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa <[email protected]> Date: Mon, 24 Aug 2026 17:29:36 +0530 Subject: [PATCH 2/2] remove duplicated code --- clang/lib/Interpreter/IncrementalParser.cpp | 24 ++++++++------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp index a30c720873abb..8be9e4f7346bb 100644 --- a/clang/lib/Interpreter/IncrementalParser.cpp +++ b/clang/lib/Interpreter/IncrementalParser.cpp @@ -223,7 +223,10 @@ bool IncrementalParser::withdrawRedecl(NamedDecl *D, NamedDecl *Prev, TranslationUnitDecl *DiscardedTU) { ASTContext &C = S.getASTContext(); - auto Unlink = [&C](auto *Latest, NamedDecl *SurvivorND) { + auto Unlink = [&C](auto *Latest, NamedDecl *SurvivorND) -> bool { + if (!Latest) + return false; + using T = std::remove_pointer_t<decltype(Latest)>; auto *Survivor = cast<T>(SurvivorND); @@ -240,10 +243,13 @@ bool IncrementalParser::withdrawRedecl(NamedDecl *D, NamedDecl *Prev, Dead->RedeclLink = Redeclarable<T>::LatestDeclLink(C); Dead = Next; } + return true; }; - if (auto *TD = dyn_cast<TagDecl>(D)) { - Unlink(TD, Prev); + if (Unlink(dyn_cast<TagDecl>(D), Prev) || + Unlink(dyn_cast<FunctionDecl>(D), Prev) || + Unlink(dyn_cast<VarDecl>(D), Prev) || + Unlink(dyn_cast<TypedefNameDecl>(D), Prev)) { // A class keeps its definition outside the redeclaration chain, in the // DefinitionData that startDefinition() hands to every redeclaration. // Unlinking leaves the survivors pointing at the discarded definition, so @@ -255,23 +261,11 @@ bool IncrementalParser::withdrawRedecl(NamedDecl *D, NamedDecl *Prev, cast<CXXRecordDecl>(R)->DefinitionData = nullptr; return true; } - if (auto *FD = dyn_cast<FunctionDecl>(D)) { - Unlink(FD, Prev); - return true; - } - if (auto *VD = dyn_cast<VarDecl>(D)) { - Unlink(VD, Prev); - return true; - } if (auto *ND = dyn_cast<NamespaceDecl>(D)) { dropContainingMembers(ND); Unlink(ND, Prev); return true; } - if (auto *TND = dyn_cast<TypedefNameDecl>(D)) { - Unlink(TND, Prev); - return true; - } if (auto *RTD = dyn_cast<RedeclarableTemplateDecl>(D)) { auto *PrevRTD = cast<RedeclarableTemplateDecl>(Prev); Unlink(RTD, Prev); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
