[clang] Evaluate non-type default template argument when it is required

Before this change a default template argument for a non-type template parameter was evaluated and checked immediately after it is met by parser. In some cases it is too early. Fixes #62224 Fixes #62596 Reviewed By: shafik, erichkeane, cor3ntin Differential Revision: https://reviews.llvm.org/D150108
llvm · May 9, 2023 · 74fd474 · 74fd474
1 parent cac4d7f
commit 74fd474
Show file tree

Hide file tree

Showing 10 changed files with 41 additions and 24 deletions.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -376,6 +376,10 @@ Bug Fixes in This Version
 - Fix crash when attempting to pass a non-pointer type as first argument of
   ``__builtin_assume_aligned``.
   (`#62305 <https://github.com/llvm/llvm-project/issues/62305>`_)
+- A default argument for a non-type template parameter is evaluated and checked
+  at the point where it is required. This fixes:
+  (`#62224 <https://github.com/llvm/llvm-project/issues/62224>`_) and
+  (`#62596 <https://github.com/llvm/llvm-project/issues/62596>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
@@ -1610,16 +1610,6 @@ NamedDecl *Sema::ActOnNonTypeTemplateParameter(Scope *S, Declarator &D,
     if (DiagnoseUnexpandedParameterPack(Default, UPPC_DefaultArgument))
       return Param;
 
-    TemplateArgument SugaredConverted, CanonicalConverted;
-    ExprResult DefaultRes = CheckTemplateArgument(
-        Param, Param->getType(), Default, SugaredConverted, CanonicalConverted,
-        CTAK_Specified);
-    if (DefaultRes.isInvalid()) {
-      Param->setInvalidDecl();
-      return Param;
-    }
-    Default = DefaultRes.get();
-
     Param->setDefaultArgument(Default);
   }
 

diff --git a/clang/test/AST/ast-dump-decl.cpp b/clang/test/AST/ast-dump-decl.cpp
@@ -455,9 +455,7 @@ namespace testClassTemplateDecl {
 // CHECK:       ClassTemplateDecl 0x{{.+}} <{{.+}}:[[@LINE-148]]:3, col:31> col:31 TestTemplateDefaultNonType
 // CHECK-NEXT:  |-NonTypeTemplateParmDecl 0x{{.+}} <col:12, col:20> col:16 'int' depth 0 index 0 I
 // CHECK-NEXT:  | `-TemplateArgument expr
-// CHECK-NEXT:  |   `-ConstantExpr 0x{{.+}} <col:20> 'int'
-// CHECK-NEXT:  |     |-value: Int 42
-// CHECK-NEXT:  |     `-IntegerLiteral 0x{{.+}} <col:20> 'int' 42
+// CHECK-NEXT:  |   `-IntegerLiteral 0x{{.+}} <col:20> 'int' 42
 // CHECK-NEXT:  `-CXXRecordDecl 0x{{.+}} <col:24, col:31> col:31 struct TestTemplateDefaultNonType
 
 // CHECK:       ClassTemplateDecl 0x{{.+}} <{{.+}}:{{.*}}:3, col:68> col:68 TestTemplateTemplateDefaultType
@@ -661,9 +659,7 @@ namespace TestNonTypeTemplateParmDecl {
 // CHECK-NEXT:   FunctionTemplateDecl
 // CHECK-NEXT:     NonTypeTemplateParmDecl{{.*}} 'int' depth 0 index 0 I
 // CHECK-NEXT:       TemplateArgument expr
-// CHECK-NEXT:         ConstantExpr{{.*}} 'int'
-// CHECK-NEXT:           value: Int 1
-// CHECK-NEXT:           IntegerLiteral{{.*}} 'int' 1
+// CHECK-NEXT:         IntegerLiteral{{.*}} 'int' 1
 // CHECK-NEXT:     NonTypeTemplateParmDecl{{.*}} 'int' depth 0 index 1 ... J
 
 namespace TestTemplateTemplateParmDecl {

diff --git a/clang/test/CXX/expr/expr.const/p3-0x.cpp b/clang/test/CXX/expr/expr.const/p3-0x.cpp
@@ -10,6 +10,7 @@ void NonConstF() {
     case nonconst: // expected-error {{case value is not a constant expression}} expected-note {{read of non-const}}
       break;
   }
+  NonConstT<> V; // expected-note {{while checking a default template argument used here}}
   return;
 }
 

diff --git a/clang/test/Index/Core/index-source.cpp b/clang/test/Index/Core/index-source.cpp
@@ -405,7 +405,7 @@ template<typename T = Cls,
 // CHECK: [[@LINE-1]]:23 | class/C++ | Cls | c:@S@Cls | <no-cgname> | Ref,RelCont | rel: 1
 // CHECK-NEXT: RelCont | TemplateDefaultValues | c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
          int x = Record::C,
-// CHECK: [[@LINE-1]]:26 | static-property/C++ | C | c:@S@Record@C | __ZN6Record1CE | Ref,Read,RelCont | rel: 1
+// CHECK: [[@LINE-1]]:26 | static-property/C++ | C | c:@S@Record@C | __ZN6Record1CE | Ref,RelCont | rel: 1
 // CHECK-NEXT: RelCont | TemplateDefaultValues | c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
 // CHECK: [[@LINE-3]]:18 | struct/C++ | Record | c:@S@Record | <no-cgname> | Ref,RelCont | rel: 1
          template <typename> class Collection = ns2::ACollectionDecl>

diff --git a/clang/test/SemaCXX/GH62596.cpp b/clang/test/SemaCXX/GH62596.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+
+struct foo {
+  static constexpr bool bar() {
+      return true;
+  }
+
+  template<bool B = bar()>
+  static constexpr bool baz() {
+      return B;
+  }
+};
+static_assert(foo::baz(), "");
+
+// expected-no-diagnostics
diff --git a/clang/test/SemaCXX/cxx2a-consteval-default-params.cpp b/clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
@@ -79,3 +79,16 @@ namespace ShouldNotCrash {
     };
     void f(A a = A()) { }
 }
+
+namespace GH62224 {
+  consteval int fwd();
+  template <int i = fwd()>
+  struct C {
+    consteval C(int = fwd()) { }
+    consteval int get() { return i; }
+  };
+
+  consteval int fwd() { return 42; }
+  C<> Val; // No error since fwd is defined already.
+  static_assert(Val.get() == 42);
+}
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
@@ -224,9 +224,7 @@ F s(0);
 // CHECK: |-NonTypeTemplateParmDecl {{.*}} 'char' depth 0 index 0
 // CHECK:   `-TemplateArgument expr
 // CHECK: |   |-inherited from NonTypeTemplateParm {{.*}} '' 'char'
-// CHECK: |   `-ConstantExpr {{.*}} 'char'
-// CHECK: |     |-value: Int 120
-// CHECK: |     `-CharacterLiteral {{.*}} 'char' 120
+// CHECK: |   `-CharacterLiteral {{.*}} 'char' 120
 // CHECK: |-TemplateTypeParmDecl {{.*}} typename depth 0 index 1 U
 // CHECK: |-ParenExpr {{.*}} 'bool'
 // CHECK: | `-CXXBoolLiteralExpr {{.*}} 'bool' false

diff --git a/clang/test/SemaTemplate/temp_arg_nontype.cpp b/clang/test/SemaTemplate/temp_arg_nontype.cpp
@@ -204,8 +204,8 @@ namespace EntityReferenced {
 }
 
 namespace PR6964 {
-  template <typename ,int, int = 9223372036854775807L > // expected-warning 2{{non-type template argument value '9223372036854775807' truncated to '-1' for template parameter of type 'int'}} \
-  // expected-note 2{{template parameter is declared here}}
+  template <typename ,int, int = 9223372036854775807L > // expected-warning {{non-type template argument value '9223372036854775807' truncated to '-1' for template parameter of type 'int'}} \
+  // expected-note {{template parameter is declared here}}
   struct as_nview { };
 
   template <typename Sequence, int I0> 

diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
@@ -1156,8 +1156,6 @@ TEST_P(ASTImporterOptionSpecificTestBase, NonTypeTemplateParmDeclDefaultArg) {
   NonTypeTemplateParmDecl *To = Import(From, Lang_CXX03);
   ASSERT_TRUE(To->hasDefaultArgument());
   Stmt *ToArg = To->getDefaultArgument();
-  ASSERT_TRUE(isa<ConstantExpr>(ToArg));
-  ToArg = *ToArg->child_begin();
   ASSERT_TRUE(isa<IntegerLiteral>(ToArg));
   ASSERT_EQ(cast<IntegerLiteral>(ToArg)->getValue().getLimitedValue(), 1U);
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -10,6 +10,7 @@ void NonConstF() { @@
         case nonconst: // expected-error {{case value is not a constant expression}} expected-note {{read of non-const}}
           break;
       }
+      NonConstT<> V; // expected-note {{while checking a default template argument used here}}
       return;
     }
@@ Expand Down @@