[PATCH] D113874: [clang] Propagate requires-clause from constructor template to implicit deduction guide

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
nridge updated this revision to Diff 404437.
nridge added a comment.
nridge retitled this revision from "[WIP] [clang] Attempt to fix crash in 
https://github.com/clangd/clangd/issues/890"; to "[clang] Propagate 
requires-clause from constructor template to implicit deduction guide".
nridge edited the summary of this revision.
nridge published this revision for review.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

Revise fix approach


Fixes https://github.com/clangd/clangd/issues/890


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113874

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/lib/Sema/SemaTemplate.cpp


Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2184,10 +2184,24 @@
         SubstArgs.push_back(SemaRef.Context.getCanonicalTemplateArgument(
             SemaRef.Context.getInjectedTemplateArg(NewParam)));
       }
+
+      // Substitute new template parameters into requires-clause if present.
+      Expr *RequiresClause = nullptr;
+      if (Expr *InnerRC = InnerParams->getRequiresClause()) {
+        MultiLevelTemplateArgumentList Args;
+        Args.setKind(TemplateSubstitutionKind::Rewrite);
+        Args.addOuterTemplateArguments(SubstArgs);
+        Args.addOuterRetainedLevel();
+        ExprResult E = SemaRef.SubstExpr(InnerRC, Args);
+        if (E.isInvalid())
+          return nullptr;
+        RequiresClause = E.getAs<Expr>();
+      }
+
       TemplateParams = TemplateParameterList::Create(
           SemaRef.Context, InnerParams->getTemplateLoc(),
           InnerParams->getLAngleLoc(), AllParams, InnerParams->getRAngleLoc(),
-          /*FIXME: RequiresClause*/ nullptr);
+          RequiresClause);
     }
 
     // If we built a new template-parameter-list, track that we need to
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -791,6 +791,29 @@
   EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
 }
 
+TEST(DiagnosticsTest, ConstrainedImplicitDeductionGuides) {
+  Annotations Header(R"cpp(
+template<char = 'x'> struct S;
+
+template<char> struct S {
+    template<typename U> requires(false) S(U);
+    template<typename U> requires(true) S(U);
+};
+
+template<typename T> S(T) -> S<>;
+  )cpp");
+  Annotations Main(R"cpp(
+    #include "a.h"
+    void foo() {
+     S(0); // Simply ::x; is enough to crash!
+    }
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ExtraArgs.push_back("-std=c++20");
+  TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+}
+
 TEST(ClangdTest, MSAsm) {
   // Parsing MS assembly tries to use the target MCAsmInfo, which we don't 
link.
   // We used to crash here. Now clang emits a diagnostic, which we filter out.



Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2184,10 +2184,24 @@
         SubstArgs.push_back(SemaRef.Context.getCanonicalTemplateArgument(
             SemaRef.Context.getInjectedTemplateArg(NewParam)));
       }
+
+      // Substitute new template parameters into requires-clause if present.
+      Expr *RequiresClause = nullptr;
+      if (Expr *InnerRC = InnerParams->getRequiresClause()) {
+        MultiLevelTemplateArgumentList Args;
+        Args.setKind(TemplateSubstitutionKind::Rewrite);
+        Args.addOuterTemplateArguments(SubstArgs);
+        Args.addOuterRetainedLevel();
+        ExprResult E = SemaRef.SubstExpr(InnerRC, Args);
+        if (E.isInvalid())
+          return nullptr;
+        RequiresClause = E.getAs<Expr>();
+      }
+
       TemplateParams = TemplateParameterList::Create(
           SemaRef.Context, InnerParams->getTemplateLoc(),
           InnerParams->getLAngleLoc(), AllParams, InnerParams->getRAngleLoc(),
-          /*FIXME: RequiresClause*/ nullptr);
+          RequiresClause);
     }
 
     // If we built a new template-parameter-list, track that we need to
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -791,6 +791,29 @@
   EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
 }
 
+TEST(DiagnosticsTest, ConstrainedImplicitDeductionGuides) {
+  Annotations Header(R"cpp(
+template<char = 'x'> struct S;
+
+template<char> struct S {
+    template<typename U> requires(false) S(U);
+    template<typename U> requires(true) S(U);
+};
+
+template<typename T> S(T) -> S<>;
+  )cpp");
+  Annotations Main(R"cpp(
+    #include "a.h"
+    void foo() {
+     S(0); // Simply ::x; is enough to crash!
+    }
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ExtraArgs.push_back("-std=c++20");
+  TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+}
+
 TEST(ClangdTest, MSAsm) {
   // Parsing MS assembly tries to use the target MCAsmInfo, which we don't link.
   // We used to crash here. Now clang emits a diagnostic, which we filter out.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits