[llvm-branch-commits] [clang] release/23.x: [clang-repl] Don't double-remove extern "C" decls from the IdResolver (#218129) (PR #218220)

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: llvmbot

<details>
<summary>Changes</summary>

Backport ec5c9182be2aa168b7c843374c5c7497dd3bef17

Requested by: @<!-- -->Vipul-Cariappa

---
Full diff: https://github.com/llvm/llvm-project/pull/218220.diff


2 Files Affected:

- (modified) clang/lib/Interpreter/IncrementalParser.cpp (+9-5) 
- (added) clang/test/Interpreter/extern-c-error-recovery.cpp (+63) 


``````````diff
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp
index f6d2779d64b2b..0f28eed888a05 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -198,6 +198,13 @@ void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) {
       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)
+      S.IdResolver.RemoveDecl(D);
+  };
+
   ExternCContextDecl *ECCD = S.getASTContext().getExternCContextDecl();
   if (StoredDeclsMap *Map = ECCD->getPrimaryContext()->getLookupPtr()) {
     for (auto &&[Key, List] : *Map) {
@@ -216,7 +223,7 @@ void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) {
       }
       for (NamedDecl *D : NamedDeclsToRemove) {
         List.remove(D);
-        S.IdResolver.RemoveDecl(D);
+        RemoveFromIdResolver(D);
       }
     }
   }
@@ -226,10 +233,7 @@ void IncrementalParser::CleanUpPTU(TranslationUnitDecl *MostRecentTU) {
     auto *ND = dyn_cast<NamedDecl>(D);
     if (!ND || ND->getDeclName().isEmpty())
       continue;
-    // Check if we need to clean up the IdResolver chain.
-    if (ND->getDeclName().getFETokenInfo() && !D->getLangOpts().ObjC &&
-        !D->getLangOpts().CPlusPlus)
-      S.IdResolver.RemoveDecl(ND);
+    RemoveFromIdResolver(ND);
   }
 }
 
diff --git a/clang/test/Interpreter/extern-c-error-recovery.cpp b/clang/test/Interpreter/extern-c-error-recovery.cpp
new file mode 100644
index 0000000000000..20b5223700007
--- /dev/null
+++ b/clang/test/Interpreter/extern-c-error-recovery.cpp
@@ -0,0 +1,63 @@
+// REQUIRES: host-supports-jit
+// RUN: cat %s | clang-repl > %t.out 2>&1
+// RUN: FileCheck %s --input-file=%t.out
+// RUN: FileCheck %s --check-prefix=NEGATIVE --input-file=%t.out
+
+// An input that declares something with C language linkage and then fails must
+// not take the interpreter down with it.
+
+extern "C" int printf(const char *, ...);
+
+// An error in the body of an extern "C" function definition.
+extern "C" void f1() { undeclared_thing; }
+// CHECK-DAG: error: use of undeclared identifier 'undeclared_thing'
+printf("alive %d\n", 1);
+// CHECK-DAG: alive 1
+
+// The same, written as an `extern "C" { ... }` block.
+extern "C" { void f2() { undeclared_thing; } }
+printf("alive %d\n", 2);
+// CHECK-DAG: alive 2
+
+// An extern "C" *variable* whose initializer fails: variables are registered
+// with the ExternCContext by a different Sema path than functions.
+extern "C" int v1 = undeclared_thing;
+printf("alive %d\n", 3);
+// CHECK-DAG: alive 3
+
+// A deleted destructor reached through a wrapper -- the shape CppInterOp's
+// generated destructor wrappers hit.
+class D { public: ~D() = delete; };
+extern "C" void g(D *p) { delete p; }
+// CHECK-DAG: error: attempt to use a deleted function
+printf("alive %d\n", 4);
+// CHECK-DAG: alive 4
+
+// A block-scope `extern` inside an extern "C" function is registered with the
+// ExternCContext but is already off the IdResolver by the time CleanUpPTU runs,
+// because its own scope popped while the input was still being parsed. Removing
+// it again therefore does not just fail to find it: with assertions off it
+// clears the identifier's chain out from under whatever else is on it.
+extern "C" void h1() { extern int fresh; undeclared_thing; } int fresh = 1;
+printf("alive %d\n", 5);
+// CHECK-DAG: alive 5
+
+// Surviving is not enough: the discarded PTU must leave nothing behind, so the
+// very same names have to be definable afterwards and the definitions have to
+// be the ones that run.
+extern "C" void f1() { printf("f1 ran\n"); }
+extern "C" void f2() { printf("f2 ran\n"); }
+extern "C" int v1 = 5;
+f1();
+f2();
+printf("v1 %d\n", v1);
+// CHECK-DAG: f1 ran
+// CHECK-DAG: f2 ran
+// CHECK-DAG: v1 5
+
+// Nothing anywhere in the session may claim the recovered definitions clash
+// with what the discarded inputs left behind.
+// NEGATIVE-NOT: error: redefinition
+// NEGATIVE-NOT: error: conflicting types
+
+%quit

``````````

</details>


https://github.com/llvm/llvm-project/pull/218220