Skip to content
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

[Clang] Fix crash when ill-formed code is treated as a deduction guide #67373

Merged
merged 4 commits into from
Oct 2, 2023

Conversation

shafik
Copy link
Collaborator

@shafik shafik commented Sep 25, 2023

In some cases where ill-formed code could be interpreted as a deduction guide we can crash because we reach an unreachable path. This fixes this issue by introducing a diagnostic instead.

Fixes: #65522

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 25, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 25, 2023

@llvm/pr-subscribers-clang

Changes

In some cases where ill-formed code could be interpreted as a deduction guide we can crash because we reach an unreachable path. This fixes this issue by introducing a diagnostic instead.

Fixes: #65522


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

3 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+3-1)
  • (modified) clang/test/SemaCXX/cxx1z-copy-omission.cpp (+9)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f4eb02fd9570c2f..a6db3f3d0105742 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5414,6 +5414,8 @@ def note_constraint_normalization_here : Note<
 def note_parameter_mapping_substitution_here : Note<
   "while substituting into concept arguments here; substitution failures not "
   "allowed in concept arguments">;
+def note_building_deduction_guide_here : Note<
+  "while building deduction guide here">;
 def note_lambda_substitution_here : Note<
   "while substituting into a lambda expression here">;
 def note_instantiation_contexts_suppressed : Note<
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 00a36696cf90450..1ed3df5a011bcb6 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1075,7 +1075,9 @@ void Sema::PrintInstantiationStack() {
           << Active->InstantiationRange;
       break;
     case CodeSynthesisContext::BuildingDeductionGuides:
-      llvm_unreachable("unexpected deduction guide in instantiation stack");
+      Diags.Report(Active->PointOfInstantiation,
+                   diag::note_building_deduction_guide_here);
+      break;
     }
   }
 }
diff --git a/clang/test/SemaCXX/cxx1z-copy-omission.cpp b/clang/test/SemaCXX/cxx1z-copy-omission.cpp
index a850cf6143cd480..819cba258915eef 100644
--- a/clang/test/SemaCXX/cxx1z-copy-omission.cpp
+++ b/clang/test/SemaCXX/cxx1z-copy-omission.cpp
@@ -171,3 +171,12 @@ namespace CtorTemplateBeatsNonTemplateConversionFn {
   Foo f(Derived d) { return d; } // expected-error {{invokes a deleted function}}
   Foo g(Derived d) { return Foo(d); } // ok, calls constructor
 }
+
+namespace GH65522 {
+template<typename A3>
+class B3 : A3 {
+  template<bool = C3<B3>()> // expected-warning 2{{use of function template name with no prior declaration in function call with explicit}}
+  B3();
+}; B3(); // expected-error {{deduction guide declaration without trailing return type}} \
+         // expected-note {{while building deduction guide here}}
+}

@shafik
Copy link
Collaborator Author

shafik commented Sep 25, 2023

I was originally going to roll-up this fix as part of: https://reviews.llvm.org/D148474 but it felt distinct enough to fix on its own and then rebase the other PR afterwards.

In some cases where ill-formed code could be interpreted as a deduction guide
we can crash because we reach an unreachable path. This fixes this issue by
introducing a diagnostic instead.

Fixes: llvm#65522
@shafik shafik force-pushed the fix_deduction_guide_instantiation_stack branch from 1877bcc to beab5db Compare September 26, 2023 20:32
Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably needs a release note.

@shafik
Copy link
Collaborator Author

shafik commented Sep 29, 2023

ping

Copy link
Collaborator

@yuanfang-chen yuanfang-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Modulo nit

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved
@shafik shafik merged commit f1fed12 into llvm:main Oct 2, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Clang] Ill-formed default argument treated as building a deduction guide causes crash
6 participants