Skip to content

Conversation

@jcsxky
Copy link
Contributor

@jcsxky jcsxky commented Apr 28, 2024

Fix #68885
When build expression from a deduced argument whose kind is Declaration and NTTPType(which declared as decltype(auto)) is deduced as a reference type, BuildExpressionFromDeclTemplateArgument just create a DeclRef. This is incorrect while we get type from the expression since we can't get the original reference type from DeclRef. Creating a SubstNonTypeTemplateParmExpr expression and make the deduction correct. Replacement expression of SubstNonTypeTemplateParmExpr just helps the deduction and may not be same with the original expression.

@jcsxky jcsxky requested a review from Endilll as a code owner April 28, 2024 06:38
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 28, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 28, 2024

@llvm/pr-subscribers-clang

Author: Qizhi Hu (jcsxky)

Changes

Fix #68885
When build expression from a deduced argument whose kind is Declaration and NTTPType(which declared as decltype(auto)) is deduced as a reference type, BuildExpressionFromDeclTemplateArgument just create a DeclRef. This is incorrect while we get type from the expression since we can't get the original reference type from DeclRef. Creating a ParenExpr expression and make the deduction correct. ParenExpr expression just helps the deduction and may not be same with the original expression.


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

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/Sema/Sema.h (+6-4)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+13-4)
  • (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+6-4)
  • (added) clang/test/SemaCXX/PR68885.cpp (+21)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a1390d6536b28c..8c1ba2a50b921b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -588,6 +588,8 @@ Bug Fixes to C++ Support
 - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235)
 - Fixed bug in which the body of a consteval lambda within a template was not parsed as within an
   immediate function context.
+- Fix a bug on template partial specialization with issue on deduction of nontype tempalte parameter
+  whose type is `decltype(auto)`. Fixes (#GH68885).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1ca523ec88c2f9..809b9c4498f697 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9249,7 +9249,8 @@ class Sema final : public SemaBase {
   void NoteTemplateParameterLocation(const NamedDecl &Decl);
 
   ExprResult BuildExpressionFromDeclTemplateArgument(
-      const TemplateArgument &Arg, QualType ParamType, SourceLocation Loc);
+      const TemplateArgument &Arg, QualType ParamType, SourceLocation Loc,
+      NamedDecl *TemplateParam = nullptr);
   ExprResult
   BuildExpressionFromNonTypeTemplateArgument(const TemplateArgument &Arg,
                                              SourceLocation Loc);
@@ -9572,9 +9573,10 @@ class Sema final : public SemaBase {
 
   bool isSameOrCompatibleFunctionType(QualType Param, QualType Arg);
 
-  TemplateArgumentLoc getTrivialTemplateArgumentLoc(const TemplateArgument &Arg,
-                                                    QualType NTTPType,
-                                                    SourceLocation Loc);
+  TemplateArgumentLoc
+  getTrivialTemplateArgumentLoc(const TemplateArgument &Arg, QualType NTTPType,
+                                SourceLocation Loc,
+                                NamedDecl *TemplateParam = nullptr);
 
   /// Get a template argument mapping the given template parameter to itself,
   /// e.g. for X in \c template<int X>, this would return an expression template
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index bbcb7c33a98579..6a2f025521ce8e 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -8438,10 +8438,9 @@ void Sema::NoteTemplateParameterLocation(const NamedDecl &Decl) {
 /// declaration and the type of its corresponding non-type template
 /// parameter, produce an expression that properly refers to that
 /// declaration.
-ExprResult
-Sema::BuildExpressionFromDeclTemplateArgument(const TemplateArgument &Arg,
-                                              QualType ParamType,
-                                              SourceLocation Loc) {
+ExprResult Sema::BuildExpressionFromDeclTemplateArgument(
+    const TemplateArgument &Arg, QualType ParamType, SourceLocation Loc,
+    NamedDecl *TemplateParam) {
   // C++ [temp.param]p8:
   //
   //   A non-type template-parameter of type "array of T" or
@@ -8508,6 +8507,16 @@ Sema::BuildExpressionFromDeclTemplateArgument(const TemplateArgument &Arg,
   } else {
     assert(ParamType->isReferenceType() &&
            "unexpected type for decl template argument");
+    if (ParamType->isLValueReferenceType())
+      if (NonTypeTemplateParmDecl *NTTP =
+              dyn_cast_if_present<NonTypeTemplateParmDecl>(TemplateParam)) {
+        QualType TemplateParamType = NTTP->getType();
+        const AutoType *AT = TemplateParamType->getAs<AutoType>();
+        if (AT && AT->isDecltypeAuto())
+          RefExpr = new (getASTContext())
+              ParenExpr(RefExpr.get()->getBeginLoc(),
+                        RefExpr.get()->getEndLoc(), RefExpr.get());
+      }
   }
 
   // At this point we should have the right value category.
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index c3815bca038554..e93f7bd842e444 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2639,7 +2639,8 @@ static bool isSameTemplateArg(ASTContext &Context,
 /// argument.
 TemplateArgumentLoc
 Sema::getTrivialTemplateArgumentLoc(const TemplateArgument &Arg,
-                                    QualType NTTPType, SourceLocation Loc) {
+                                    QualType NTTPType, SourceLocation Loc,
+                                    NamedDecl *TemplateParam) {
   switch (Arg.getKind()) {
   case TemplateArgument::Null:
     llvm_unreachable("Can't get a NULL template argument here");
@@ -2651,7 +2652,8 @@ Sema::getTrivialTemplateArgumentLoc(const TemplateArgument &Arg,
   case TemplateArgument::Declaration: {
     if (NTTPType.isNull())
       NTTPType = Arg.getParamTypeForDecl();
-    Expr *E = BuildExpressionFromDeclTemplateArgument(Arg, NTTPType, Loc)
+    Expr *E = BuildExpressionFromDeclTemplateArgument(Arg, NTTPType, Loc,
+                                                      TemplateParam)
                   .getAs<Expr>();
     return TemplateArgumentLoc(TemplateArgument(E), E);
   }
@@ -2718,8 +2720,8 @@ static bool ConvertDeducedTemplateArgument(
     // Convert the deduced template argument into a template
     // argument that we can check, almost as if the user had written
     // the template argument explicitly.
-    TemplateArgumentLoc ArgLoc =
-        S.getTrivialTemplateArgumentLoc(Arg, QualType(), Info.getLocation());
+    TemplateArgumentLoc ArgLoc = S.getTrivialTemplateArgumentLoc(
+        Arg, QualType(), Info.getLocation(), Param);
 
     // Check the template argument, converting it as necessary.
     return S.CheckTemplateArgument(
diff --git a/clang/test/SemaCXX/PR68885.cpp b/clang/test/SemaCXX/PR68885.cpp
new file mode 100644
index 00000000000000..4ff95771e2caf5
--- /dev/null
+++ b/clang/test/SemaCXX/PR68885.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+
+// expected-no-diagnostics
+
+template <decltype(auto) a>
+struct S {
+    static constexpr int i = 42;
+};
+
+template <decltype(auto) a> requires true
+struct S<a> {
+    static constexpr int i = 0;
+};
+
+static constexpr int a = 0;
+
+void test() {
+    static_assert(S<a>::i == 0);
+    static_assert(S<(a)>::i == 0);
+    static_assert(S<((a))>::i == 0);
+}

Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

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

Sema.h changes look good to me.

const AutoType *AT = TemplateParamType->getAs<AutoType>();
if (AT && AT->isDecltypeAuto())
RefExpr = new (getASTContext())
ParenExpr(RefExpr.get()->getBeginLoc(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try to preserve the references using SubstNonTypeTemplateParmExprs just like what @cor3ntin said? This looks a bit odd to me...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks for your remind! SubstNonTypeTemplateParmExpr is more suitable here.

@github-actions
Copy link

github-actions bot commented Apr 28, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@jcsxky jcsxky force-pushed the fix-68885 branch 2 times, most recently from cb7acd7 to 9470969 Compare April 29, 2024 02:23
@cor3ntin cor3ntin changed the title [Clang][Sema] Fix a bug on template partial specialization with issue on deduction of nontype tempalte parameter [Clang][Sema] Fix a bug on template partial specialization with issue on deduction of nontype template parameter Apr 29, 2024
… on deduction of nontype tempalte parameter
@jcsxky jcsxky merged commit a413c56 into llvm:main Apr 30, 2024
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.

NTTP references in requires clauses

5 participants