Author: Vipul Cariappa Date: 2026-09-23T10:53:43Z New Revision: 37e2844bc4681c69f61cbb55376a29ae0445c18c
URL: https://github.com/llvm/llvm-project/commit/37e2844bc4681c69f61cbb55376a29ae0445c18c DIFF: https://github.com/llvm/llvm-project/commit/37e2844bc4681c69f61cbb55376a29ae0445c18c.diff LOG: [clang-repl] Keep earlier declarations alive when an input fails (#218149) Fixes #201844 Added: clang/test/Interpreter/failed-input-keeps-redecls.cpp Modified: clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/Redeclarable.h clang/lib/Interpreter/IncrementalParser.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 6941f0179647a..1ebc8a7191bde 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -257,6 +257,7 @@ class CXXBaseSpecifier { /// Represents a C++ struct/union/class. class CXXRecordDecl : public RecordDecl { friend class ASTDeclMerger; + friend class ASTDeclUnmerger; friend class ASTDeclReader; friend class ASTDeclWriter; friend class ASTNodeImporter; diff --git a/clang/include/clang/AST/Redeclarable.h b/clang/include/clang/AST/Redeclarable.h index 35911ee2f7d16..fe2648aa58301 100644 --- a/clang/include/clang/AST/Redeclarable.h +++ b/clang/include/clang/AST/Redeclarable.h @@ -186,6 +186,7 @@ class Redeclarable { public: friend class ASTDeclMerger; + friend class ASTDeclUnmerger; friend class ASTDeclReader; friend class ASTDeclWriter; friend class IncrementalParser; diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp index 12beb542572d7..3cfabb5e5daa1 100644 --- a/clang/lib/Interpreter/IncrementalParser.cpp +++ b/clang/lib/Interpreter/IncrementalParser.cpp @@ -15,7 +15,10 @@ #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/AST/DeclVisitor.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Interpreter/PartialTranslationUnit.h" #include "clang/Parse/Parser.h" @@ -191,69 +194,139 @@ void IncrementalParser::withdrawMostRecentTU( C.TUDecl = Prev; } -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; - 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 - RemoveAll = false; - } - if (LLVM_LIKELY(RemoveAll)) { - KeysToErase.push_back(Key); - } else { - for (NamedDecl *D : NamedDeclsToRemove) - List.remove(D); - } +/// Removes decls introduced in the discarding PTU and restores the +/// redeclaration chain to previous state. +class ASTDeclUnmerger : public DeclVisitor<ASTDeclUnmerger> { + Sema &S; + TranslationUnitDecl *DiscardedTU; + + template <typename DeclT> void withdraw(Redeclarable<DeclT> *DBase) { + if (NamedDecl *Prev = findSurvivor(static_cast<DeclT *>(DBase))) + unlinkRedeclChain(S.getASTContext(), DBase, Prev); + } + + /// The newest declaration of whatever D redeclares that still lives outside + /// the DiscardedTU, or null if DiscardedTU introduced the name. + NamedDecl *findSurvivor(NamedDecl *D) const { + for (Decl *Prev = D->getPreviousDecl(); Prev; + Prev = Prev->getPreviousDecl()) + if (Prev->getTranslationUnitDecl() != DiscardedTU) + return dyn_cast<NamedDecl>(Prev); + return nullptr; + } + + template <typename DeclT> + void unlinkRedeclChain(ASTContext &C, Redeclarable<DeclT> *DBase, + NamedDecl *PrevND) { + auto *Latest = static_cast<DeclT *>(DBase); + auto *Survivor = cast<DeclT>(PrevND); + + // 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 withdrawn declaration a chain of its own. + for (DeclT *Dead = Latest; Dead != Survivor;) { + DeclT *Next = Dead->getPreviousDecl(); + Dead->First = Dead; + Dead->RedeclLink = Redeclarable<DeclT>::LatestDeclLink(C); + Dead = Next; } - for (DeclarationName Key : KeysToErase) - Map->erase(Key); } - // Check if we need to clean up the IdResolver chain. - auto RemoveFromIdResolver = [&](NamedDecl *D) { - if (D->getDeclName().getFETokenInfo() && !D->getLangOpts().ObjC && - !D->getLangOpts().CPlusPlus) + /// Remove entry from "C"'s lookup tables + void removeFromLookups(NamedDecl *D) { + if (D->getDeclName().isEmpty()) + return; + + if (D->getDeclName().isIdentifier() && D->getDeclName().getFETokenInfo() && + !D->getLangOpts().ObjC && !D->getLangOpts().CPlusPlus) S.IdResolver.RemoveDecl(D); - }; - - ExternCContextDecl *ECCD = S.getASTContext().getExternCContextDecl(); - if (StoredDeclsMap *Map = ECCD->getPrimaryContext()->getLookupPtr()) { - for (auto &&[Key, List] : *Map) { - DeclContextLookupResult R = List.getLookupResult(); - llvm::SmallVector<NamedDecl *, 4> NamedDeclsToRemove; - for (NamedDecl *D : R) { - // Implicitly generated C decl is not attached to the current TU but - // lexically attached to the recent TU, so we need to check the lexical - // context. - DeclContext *LDC = D->getLexicalDeclContext(); - while (LDC && !isa<TranslationUnitDecl>(LDC)) - LDC = LDC->getLexicalParent(); - TranslationUnitDecl *TopTU = cast_or_null<TranslationUnitDecl>(LDC); - if (TopTU == MostRecentTU) - NamedDeclsToRemove.push_back(D); - } - for (NamedDecl *D : NamedDeclsToRemove) { - List.remove(D); - RemoveFromIdResolver(D); - } + + ExternCContextDecl *ECCD = S.getASTContext().getExternCContextDecl(); + if (StoredDeclsMap *Map = ECCD->getPrimaryContext()->getLookupPtr()) { + auto It = Map->find(D->getDeclName()); + if (It != Map->end()) + It->second.remove(D); + } + } + + /// Remove Decls defined in this DC from the lookup table + /// and restore the redeclaration chain to previous state + void VisitDeclContext(DeclContext *DC) { + llvm::SmallVector<Decl *, 8> Members(DC->decls()); + llvm::SmallVector<NamedDecl *, 8> Survivors; + for (Decl *M : Members) { + if (auto *ND = dyn_cast<NamedDecl>(M)) + if (NamedDecl *Prev = findSurvivor(ND)) + Survivors.push_back(Prev); + Visit(M); // restore redecls + DC->removeDecl(M); // remove from lookup + if (auto *ND = dyn_cast<NamedDecl>(M)) + removeFromLookups(ND); } + + // Restore lookup for the surviving predecessor + // of any removed decl that had a surviving predecessor + DeclContext *Primary = DC->getPrimaryContext(); + for (NamedDecl *Prev : Survivors) + Primary->makeDeclVisibleInContext(Prev); + } + +public: + ASTDeclUnmerger(Sema &S, TranslationUnitDecl *DiscardedTU) + : S(S), DiscardedTU(DiscardedTU) {} + + void VisitDecl(Decl *D) { + if (auto *DC = dyn_cast<DeclContext>(D)) + VisitDeclContext(DC); + } + + void VisitFunctionDecl(FunctionDecl *D) { withdraw(D); } + void VisitNamespaceAliasDecl(NamespaceAliasDecl *D) { withdraw(D); } + void VisitTypedefNameDecl(TypedefNameDecl *D) { withdraw(D); } + void VisitUsingShadowDecl(UsingShadowDecl *D) { withdraw(D); } + void VisitVarDecl(VarDecl *D) { withdraw(D); } + + void VisitTagDecl(TagDecl *D) { + NamedDecl *Prev = findSurvivor(D); + if (!Prev) + return; + unlinkRedeclChain(S.getASTContext(), D, Prev); + + // A class definition is kept in DefinitionData outside the + // redeclaration chain + auto *RD = dyn_cast<CXXRecordDecl>(Prev); + if (!RD) + return; + if (CXXRecordDecl *Def = RD->getDefinition(); + Def && Def->getTranslationUnitDecl() == DiscardedTU) + for (auto *R : RD->redecls()) + cast<CXXRecordDecl>(R)->DefinitionData = nullptr; } - for (Decl *D : MostRecentTU->decls()) { - auto *ND = dyn_cast<NamedDecl>(D); - if (!ND || ND->getDeclName().isEmpty()) - continue; - RemoveFromIdResolver(ND); + void VisitRedeclarableTemplateDecl(RedeclarableTemplateDecl *D) { + withdraw(D); + Visit(D->getTemplatedDecl()); } + void VisitNamespaceDecl(NamespaceDecl *D) { + // Handle cases of nested redeclarations like: + // PTU1: namespace outer { namespace ns { class Foo; } } + // PTU2: namespace outer { namespace ns { class Foo { ... }; error; } } + // Foo's redeclaration needs to be restored + VisitDeclContext(D); + withdraw(D); + } + + void VisitTranslationUnitDecl(TranslationUnitDecl *D) { VisitDeclContext(D); } +}; + +void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) { + ASTDeclUnmerger(S, MostRecentTU).Visit(MostRecentTU); + // Lookup alone is not enough: the redeclaration chain still reaches these. withdrawMostRecentTU(MostRecentTU); } 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..1e582220ea8d7 --- /dev/null +++ b/clang/test/Interpreter/failed-input-keeps-redecls.cpp @@ -0,0 +1,102 @@ +// 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 + +// Namespace alias and using declaration. +namespace Deep { int v = 11; void g() { printf("Deep::g\n"); } } +namespace Al = Deep; +namespace Al = Deep; int e8 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +printf("Al::v = %d\n", Al::v); +// CHECK-DAG: Al::v = 11 + +using Deep::g; +using Deep::g; int e9 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +g(); +// CHECK-DAG: Deep::g + +// A member of a re-opened namespace is a redeclaration on its own. +namespace ns { class Foo; } +namespace ns { class Foo { public: int v; }; int e10 = undeclared_thing; } +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +ns::Foo *fp = nullptr; printf("ns::Foo reachable %d\n", fp == nullptr); +// CHECK-DAG: ns::Foo reachable 1 +namespace ns { class Foo { public: int v; int w; }; } +ns::Foo foo; foo.v = 1; foo.w = 2; printf("foo = %d %d\n", foo.v, foo.w); +// CHECK-DAG: foo = 1 2 + +namespace ns { void h(); } +namespace ns { void h() { printf("h discarded\n"); } int e11 = undeclared_thing; } +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +namespace ns { void h() { printf("h kept\n"); } } +ns::h(); +// CHECK-DAG: h kept +// NEG-NOT: {{^}}h discarded + +// The same, one namespace deeper: the inner namespace is itself a member of +// the outer one. +namespace outer { namespace inner { class Bar; } } +namespace outer { namespace inner { class Bar { public: int v; }; } int e12 = undeclared_thing; } +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +outer::inner::Bar *bp = nullptr; printf("outer::inner::Bar reachable %d\n", bp == nullptr); +// CHECK-DAG: outer::inner::Bar reachable 1 + +// Anonymous namespace +namespace { int anon_v = 11; } int e13 = undeclared_thing; +// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing' +namespace { int anon_v = 22; } +printf("anon_v = %d\n", anon_v); +// CHECK-DAG: anon_v = 22 + +%quit _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
