[clang-repl] Fix PCH with delayed template parsing

[hahnjo]

When instantiating a delayed template, the recorded token stream is passed to Parser::ParseLateTemplatedFuncDef which will append the current token "so it doesn't get lost". With incremental extensions enabled, this is repl_input_end which subsequently needs support for (de)serialization.

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Jonas Hahnfeld (hahnjo)

Changes

When instantiating a delayed template, the recorded token stream is passed to Parser::ParseLateTemplatedFuncDef which will append the current token "so it doesn't get lost". With incremental extensions enabled, this is repl_input_end which subsequently needs support for (de)serialization.

---

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

3 Files Affected:

• (modified) clang/lib/Serialization/ASTReader.cpp (+1)
• (modified) clang/lib/Serialization/ASTWriter.cpp (+1)
• (added) clang/test/Interpreter/delayed-template-parsing-pch.cpp (+25)

diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 511e2df7ad3230..fa9b815239dbb6 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1887,6 +1887,7 @@ Token ASTReader::ReadToken(ModuleFile &M, const RecordDataImpl &Record,
     case tok::annot_pragma_unused:
     case tok::annot_pragma_openacc:
     case tok::annot_pragma_openacc_end:
+    case tok::annot_repl_input_end:
       break;
     default:
       llvm_unreachable("missing deserialization code for annotation token");
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 1455f8e4145cb8..5cfb98c2a1060a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4734,6 +4734,7 @@ void ASTWriter::AddToken(const Token &Tok, RecordDataImpl &Record) {
     case tok::annot_pragma_unused:
     case tok::annot_pragma_openacc:
     case tok::annot_pragma_openacc_end:
+    case tok::annot_repl_input_end:
       break;
     default:
       llvm_unreachable("missing serialization code for annotation token");
diff --git a/clang/test/Interpreter/delayed-template-parsing-pch.cpp b/clang/test/Interpreter/delayed-template-parsing-pch.cpp
new file mode 100644
index 00000000000000..f3bd4649ed0345
--- /dev/null
+++ b/clang/test/Interpreter/delayed-template-parsing-pch.cpp
@@ -0,0 +1,25 @@
+// Test the setup without incremental extensions first
+// RUN: %clang_cc1 -std=c++17 -fdelayed-template-parsing -fpch-instantiate-templates %s -emit-pch -o %t.pch -verify
+// RUN: %clang_cc1 -std=c++17 -fdelayed-template-parsing -include-pch %t.pch %s -verify
+
+// RUN: %clang_cc1 -std=c++17 -fdelayed-template-parsing -fincremental-extensions -fpch-instantiate-templates %s -emit-pch -o %t.incremental.pch -verify
+// RUN: %clang_cc1 -std=c++17 -fdelayed-template-parsing -fincremental-extensions -include-pch %t.incremental.pch %s -verify
+
+// expected-no-diagnostics
+
+#ifndef PCH
+#define PCH
+
+// Have one template that is instantiated in the PCH (via the passed option
+// -fpch-instantiate-templates) and then serialized
+template <typename T> T ft1() { return 0; }
+inline int f1() { return ft1<int>(); }
+
+// Have a second late-parsed template that needs to be deserialized
+template <typename T> T ft2() { return 0; }
+
+#else
+
+int f2() { return ft2<int>(); }
+
+#endif

[ChuanqiXu9]

Looks not bad. Let's land it after the CI gets green.

BTW, the delayed template parsing is a deprecated technique. Both clang and MSVC won't enable this after C++20 and we think it is the root of many bugs.

[hahnjo]

BTW, the delayed template parsing is a deprecated technique. Both clang and MSVC won't enable this after C++20 and we think it is the root of many bugs.

I agree. It was needed in the past to parse the MSVC stdlib, let's check if we still need it: root-project/root#16222

[hahnjo]

BTW, the delayed template parsing is a deprecated technique. Both clang and MSVC won't enable this after C++20 and we think it is the root of many bugs.

I agree. It was needed in the past to parse the MSVC stdlib, let's check if we still need it: root-project/root#16222

Update on this: testing shows that we don't need it anymore, so we turned it off. I guess we still want to merge the fix proposed in this PR regardless? @ChuanqiXu9

[ChuanqiXu9]

BTW, the delayed template parsing is a deprecated technique. Both clang and MSVC won't enable this after C++20 and we think it is the root of many bugs.

I agree. It was needed in the past to parse the MSVC stdlib, let's check if we still need it: root-project/root#16222

Update on this: testing shows that we don't need it anymore, so we turned it off. I guess we still want to merge the fix proposed in this PR regardless? @ChuanqiXu9

At least it is good for the pre-c++20 codes : )

[vgvassilev]

Lgtm!