Skip to content

Backport: [clang] Serialization: support hashing null template arguments #141957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

mizvekov
Copy link
Contributor

@mizvekov mizvekov commented May 29, 2025

When performing overload resolution during code completion, clang will allow incomplete substitutions in more places than would be allowed for valid code, because for completion to work well, it needs clang to keep going so it can explore the space of possibilities.

Notably, we accept instantiating declarations will null template arguments, and this works fine, except that when lazily loading serialized templated declarations, the template argument hasher assumes null arguments can't be used.

This patch makes the hasher happily accept that.

Fixes #139019

When performing overload resolution during code completion,
clang will allow incomplete substitutions in more places
than would be allowed for valid code, because for completion to work well,
it needs clang to keep going so it can explore the space of possibilities.

Notably, we accept instantiating declarations will null template arguments,
and this works fine, except that when lazily loading serialzied templated
declarations, the template argument hasher assumes null arguments can't
be used.

This patch makes the hasher happily accept that.

Fixes #139019
@mizvekov mizvekov self-assigned this May 29, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels May 29, 2025
@llvmbot
Copy link
Member

llvmbot commented May 29, 2025

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

When performing overload resolution during code completion, clang will allow incomplete substitutions in more places than would be allowed for valid code, because for completion to work well, it needs clang to keep going so it can explore the space of possibilities.

Notably, we accept instantiating declarations will null template arguments, and this works fine, except that when lazily loading serialzied templated declarations, the template argument hasher assumes null arguments can't be used.

This patch makes the hasher happily accept that.

Fixes #139019


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Serialization/TemplateArgumentHasher.cpp (+3-1)
  • (added) clang/test/CodeCompletion/GH139019.cpp (+26)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7b84210fddab3..262bf4e3d4f5b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1118,6 +1118,7 @@ Miscellaneous Clang Crashes Fixed
 
 - Fixed a crash when an unscoped enumeration declared by an opaque-enum-declaration within a class template
   with a dependent underlying type is subject to integral promotion. (#GH117960)
+- Fix code completion crash involving PCH serialzied templates. (#GH139019)
 
 OpenACC Specific Changes
 ------------------------
diff --git a/clang/lib/Serialization/TemplateArgumentHasher.cpp b/clang/lib/Serialization/TemplateArgumentHasher.cpp
index 598f098f526d0..5fd6941256fe2 100644
--- a/clang/lib/Serialization/TemplateArgumentHasher.cpp
+++ b/clang/lib/Serialization/TemplateArgumentHasher.cpp
@@ -65,7 +65,9 @@ void TemplateArgumentHasher::AddTemplateArgument(TemplateArgument TA) {
 
   switch (Kind) {
   case TemplateArgument::Null:
-    llvm_unreachable("Expected valid TemplateArgument");
+    // These can occur in incomplete substitutions performed with code
+    // completion (see PartialOverloading).
+    break;
   case TemplateArgument::Type:
     AddQualType(TA.getAsType());
     break;
diff --git a/clang/test/CodeCompletion/GH139019.cpp b/clang/test/CodeCompletion/GH139019.cpp
new file mode 100644
index 0000000000000..fed35b38362a1
--- /dev/null
+++ b/clang/test/CodeCompletion/GH139019.cpp
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/test.hpp -emit-pch -o %t/1.pch
+// RUN: %clang_cc1 -std=c++20 %t/test.cpp -include-pch %t/1.pch -code-completion-at=%t/test.cpp:7:17
+
+//--- test.hpp
+#pragma once
+class provider_t
+{
+  public:
+    template<class T>
+    void emit(T *data)
+    {}
+};
+
+//--- test.cpp
+#include "test.hpp"
+
+void test()
+{
+    provider_t *focus;
+    void *data;
+    focus->emit(&data);
+}

@llvmbot
Copy link
Member

llvmbot commented May 29, 2025

@llvm/pr-subscribers-clang-modules

Author: Matheus Izvekov (mizvekov)

Changes

When performing overload resolution during code completion, clang will allow incomplete substitutions in more places than would be allowed for valid code, because for completion to work well, it needs clang to keep going so it can explore the space of possibilities.

Notably, we accept instantiating declarations will null template arguments, and this works fine, except that when lazily loading serialzied templated declarations, the template argument hasher assumes null arguments can't be used.

This patch makes the hasher happily accept that.

Fixes #139019


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Serialization/TemplateArgumentHasher.cpp (+3-1)
  • (added) clang/test/CodeCompletion/GH139019.cpp (+26)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7b84210fddab3..262bf4e3d4f5b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1118,6 +1118,7 @@ Miscellaneous Clang Crashes Fixed
 
 - Fixed a crash when an unscoped enumeration declared by an opaque-enum-declaration within a class template
   with a dependent underlying type is subject to integral promotion. (#GH117960)
+- Fix code completion crash involving PCH serialzied templates. (#GH139019)
 
 OpenACC Specific Changes
 ------------------------
diff --git a/clang/lib/Serialization/TemplateArgumentHasher.cpp b/clang/lib/Serialization/TemplateArgumentHasher.cpp
index 598f098f526d0..5fd6941256fe2 100644
--- a/clang/lib/Serialization/TemplateArgumentHasher.cpp
+++ b/clang/lib/Serialization/TemplateArgumentHasher.cpp
@@ -65,7 +65,9 @@ void TemplateArgumentHasher::AddTemplateArgument(TemplateArgument TA) {
 
   switch (Kind) {
   case TemplateArgument::Null:
-    llvm_unreachable("Expected valid TemplateArgument");
+    // These can occur in incomplete substitutions performed with code
+    // completion (see PartialOverloading).
+    break;
   case TemplateArgument::Type:
     AddQualType(TA.getAsType());
     break;
diff --git a/clang/test/CodeCompletion/GH139019.cpp b/clang/test/CodeCompletion/GH139019.cpp
new file mode 100644
index 0000000000000..fed35b38362a1
--- /dev/null
+++ b/clang/test/CodeCompletion/GH139019.cpp
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/test.hpp -emit-pch -o %t/1.pch
+// RUN: %clang_cc1 -std=c++20 %t/test.cpp -include-pch %t/1.pch -code-completion-at=%t/test.cpp:7:17
+
+//--- test.hpp
+#pragma once
+class provider_t
+{
+  public:
+    template<class T>
+    void emit(T *data)
+    {}
+};
+
+//--- test.cpp
+#include "test.hpp"
+
+void test()
+{
+    provider_t *focus;
+    void *data;
+    focus->emit(&data);
+}

@nikic nikic added this to the LLVM 20.X Release milestone May 30, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status May 30, 2025
@nikic nikic moved this from Needs Triage to Needs Merge in LLVM Release Status May 30, 2025
@tstellar tstellar merged commit 7759bb5 into release/20.x Jun 3, 2025
19 checks passed
@tstellar tstellar deleted the users/mizvekov/GH139019 branch June 3, 2025 21:39
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

5 participants