[clang] [clang-repl] Set up executor implicitly to account for init PTUs (PR #84758)

Stefan Gränitz via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 12 04:38:37 PDT 2024


================
@@ -14,7 +14,7 @@ struct A { int a; A(int a) : a(a) {} virtual ~A(); };
 // PartialTranslationUnit.
 inline A::~A() { printf("~A(%d)\n", a); }
 
-// Create one instance with new and delete it.
+// Create one instance with new and delete it. We crash here now:
 A *a1 = new A(1);
----------------
weliveindetail wrote:

> Do we even have initial PTUs in the default case?

Well, this test passes, if I comment out the for loop in the ctor that executes initial PTUs:
https://github.com/llvm/llvm-project/pull/84758/files#diff-b8484f1fc5b057f146ed5d9b6e2cd47c3f6f5ae879c7a0eee44f0a272581a88cR250-R254

> Also the minimal reproducer shows a more general version where the virtual destructor is actually defined inline (https://github.com/llvm/llvm-project/commit/c861d32d7c2791bdc058d9d9fbaecc1c2f07b8c7 addresses the case where it is out-of-line, which is special due to key virtual functions).

Oh that's a good note, I had not considered the difference yet and actually they have different backtraces. Eventually, they both reach the same VTablePtr code though.

> I could not see the stack trace on osx. Can you paste it here?

Here is a diff (inline left, out-of-line right):
<img width="2081" alt="Screenshot 2024-03-12 at 11 12 36" src="https://github.com/llvm/llvm-project/assets/7307454/41b64f92-83ea-4f82-b983-0b05e32ace42">

> So if that breaks entirely (which is critical for us), I'm personally not ok with just `XFAIL`ing it to land another change...

What breaks here is the parser and this patch doesn't even touch it. Not sure I am missing something, but it seems that it triggers a bug that always existed and just didn't show up so far.

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


More information about the cfe-commits mailing list