ChuanqiXu updated this revision to Diff 409507.
ChuanqiXu added a comment.

Use ODRHash to simplify code.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118034/new/

https://reviews.llvm.org/D118034

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
  clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
  clang/test/Modules/Inputs/redundant-template-default-arg2/foo.cppm
  clang/test/Modules/Inputs/redundant-template-default-arg3/foo.cppm
  clang/test/Modules/Inputs/redundant-template-default-arg3/foo.h
  clang/test/Modules/Inputs/redundant-template-default-arg3/foo_bad.h
  clang/test/Modules/redundant-template-default-arg.cpp
  clang/test/Modules/redundant-template-default-arg2.cpp
  clang/test/Modules/redundant-template-default-arg3.cpp

Index: clang/test/Modules/redundant-template-default-arg3.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/redundant-template-default-arg3.cpp
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1  -std=c++20 %S/Inputs/redundant-template-default-arg3/foo.cppm -I%S/Inputs/redundant-template-default-arg3/. -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1  -fprebuilt-module-path=%t -std=c++20 %s -I%S/Inputs/redundant-template-default-arg3/. -fsyntax-only -verify
+
+import foo;
+#include "foo_bad.h"
+
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:1 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:1 {{previous default template argument defined in module foo.<global>}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:4 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:4 {{previous default template argument defined in module foo.<global>}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:10 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:10 {{previous default template argument defined in module foo.<global>}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:17 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:13 {{previous default template argument defined in module foo.<global>}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:23 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:16 {{previous default template argument defined in module foo.<global>}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:27 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:20 {{previous default template argument defined in module foo.<global>}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:31 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:24 {{previous default template argument defined in module foo.<global>}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:34 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:27 {{previous default template argument defined in module foo.<global>}}
+// expected-error@Inputs/redundant-template-default-arg3/foo_bad.h:40 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note@Inputs/redundant-template-default-arg3/foo.h:33 {{previous default template argument defined in module foo.<global>}}
Index: clang/test/Modules/redundant-template-default-arg2.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/redundant-template-default-arg2.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1  -std=c++20 %S/Inputs/redundant-template-default-arg2/foo.cppm -I%S/Inputs/redundant-template-default-arg2/. -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1  -fprebuilt-module-path=%t -std=c++20 %s -fsyntax-only -verify
+import foo;
+template <typename T = int>
+T v; // expected-error {{declaration of 'v' in the global module follows declaration in module foo}}
+     // expected-note@Inputs/redundant-template-default-arg2/foo.cppm:3 {{previous declaration is here}}
+
+template <int T = 8>
+int v2; // expected-error {{declaration of 'v2' in the global module follows declaration in module foo}}
+        // expected-note@Inputs/redundant-template-default-arg2/foo.cppm:6 {{previous declaration is here}}
+
+template <typename T>
+class my_array {}; // expected-error {{redefinition of 'my_array'}}
+                   // expected-note@Inputs/redundant-template-default-arg2/foo.cppm:9 {{previous definition is here}}
+
+template <template <typename> typename C = my_array>
+int v3; // expected-error {{declaration of 'v3' in the global module follows declaration in module foo}}
+        // expected-note@Inputs/redundant-template-default-arg2/foo.cppm:12 {{previous declaration is here}}
Index: clang/test/Modules/redundant-template-default-arg.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/redundant-template-default-arg.cpp
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1  -std=c++20 %S/Inputs/redundant-template-default-arg/foo.cppm -I%S/Inputs/redundant-template-default-arg/. -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1  -fprebuilt-module-path=%t -std=c++20 %s -I%S/Inputs/redundant-template-default-arg/. -fsyntax-only -verify
+// expected-no-diagnostics
+import foo;
+#include "foo.h"
Index: clang/test/Modules/Inputs/redundant-template-default-arg3/foo_bad.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg3/foo_bad.h
@@ -0,0 +1,41 @@
+template <typename T = double>
+T v;
+
+template <int T = 9>
+int v2;
+
+template <typename T>
+class others_array {};
+
+template <template <typename> typename C = others_array>
+int v3;
+
+static int a;
+consteval int *getIntPtr() {
+  return &a;
+}
+template <typename T, int *i = getIntPtr()>
+T v4;
+
+consteval void *getVoidPtr() {
+  return &a;
+}
+template <typename T, T *i = getVoidPtr()>
+T v5;
+
+inline int a_ = 43;
+template <typename T, int *i = &a_>
+T v6;
+
+inline int b_ = 43;
+template <typename T, T *i = &b_>
+T v7;
+
+template <int T = -1>
+int v8;
+
+consteval int getInt2() {
+  return 55;
+}
+template <int T = getInt2()>
+int v9;
Index: clang/test/Modules/Inputs/redundant-template-default-arg3/foo.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg3/foo.h
@@ -0,0 +1,34 @@
+template <typename T = int>
+T v;
+
+template <int T = 8>
+int v2;
+
+template <typename T>
+class my_array {};
+
+template <template <typename> typename C = my_array>
+int v3;
+
+template <typename T, int *i = nullptr>
+T v4;
+
+template <typename T, T *i = nullptr>
+T v5;
+
+inline int a = 43;
+template <typename T, int *i = &a>
+T v6;
+
+inline int b = 43;
+template <typename T, T *i = &b>
+T v7;
+
+template <int T = (3 > 2)>
+int v8;
+
+consteval int getInt() {
+  return 55;
+}
+template <int T = getInt()>
+int v9;
Index: clang/test/Modules/Inputs/redundant-template-default-arg3/foo.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg3/foo.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module foo;
Index: clang/test/Modules/Inputs/redundant-template-default-arg2/foo.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg2/foo.cppm
@@ -0,0 +1,12 @@
+export module foo;
+export template <typename T = int>
+T v;
+
+export template <int T = 8>
+int v2;
+
+export template <typename T>
+class my_array {};
+
+export template <template <typename> typename C = my_array>
+int v3;
Index: clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
@@ -0,0 +1,37 @@
+template <typename T>
+T u;
+
+template <typename T = int>
+T v;
+
+template <int T = 8>
+int v2;
+
+template <typename T>
+class my_array {};
+
+template <template <typename> typename C = my_array>
+int v3;
+
+template <typename T, int *i = nullptr>
+T v4;
+
+template <typename T, T *i = nullptr>
+T v5;
+
+inline int a = 43;
+template <typename T, int *i = &a>
+T v6;
+
+inline int b = 43;
+template <typename T, T *i = &b>
+T v7;
+
+template <int T = (3 > 2)>
+int v8;
+
+consteval int getInt() {
+  return 55;
+}
+template <int T = getInt()>
+int v9;
Index: clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module foo;
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/ODRHash.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/Basic/Builtins.h"
@@ -2624,6 +2625,46 @@
   return false;
 }
 
+static bool hasSameDefaultTemplateArgument(NamedDecl *X, NamedDecl *Y) {
+  ASTContext &C = X->getASTContext();
+  // If the type parameter isn't the same already, we don't need to check the
+  // default argument further.
+  if (!C.isSameTemplateParameter(X, Y))
+    return false;
+
+  if (auto *TTPX = dyn_cast<TemplateTypeParmDecl>(X)) {
+    auto *TTPY = cast<TemplateTypeParmDecl>(Y);
+    if (!TTPX->hasDefaultArgument() || !TTPY->hasDefaultArgument())
+      return false;
+
+    return C.hasSameType(TTPX->getDefaultArgument(),
+                         TTPY->getDefaultArgument());
+  }
+
+  if (auto *NTTPX = dyn_cast<NonTypeTemplateParmDecl>(X)) {
+    auto *NTTPY = cast<NonTypeTemplateParmDecl>(Y);
+    if (!NTTPX->hasDefaultArgument() || !NTTPY->hasDefaultArgument())
+      return false;
+
+    Expr *DefaultArgumentX = NTTPX->getDefaultArgument()->IgnoreImpCasts();
+    Expr *DefaultArgumentY = NTTPY->getDefaultArgument()->IgnoreImpCasts();
+    ODRHash X, Y;
+    X.AddStmt(DefaultArgumentX);
+    Y.AddStmt(DefaultArgumentY);
+    return X.CalculateHash() == Y.CalculateHash();
+  }
+
+  auto *TTPX = cast<TemplateTemplateParmDecl>(X);
+  auto *TTPY = cast<TemplateTemplateParmDecl>(Y);
+
+  if (!TTPX->hasDefaultArgument() || !TTPY->hasDefaultArgument())
+    return false;
+
+  const TemplateArgument &TAX = TTPX->getDefaultArgument().getArgument();
+  const TemplateArgument &TAY = TTPY->getDefaultArgument().getArgument();
+  return C.hasSameTemplateName(TAX.getAsTemplate(), TAY.getAsTemplate());
+}
+
 /// Checks the validity of a template parameter list, possibly
 /// considering the template parameter list from a previous
 /// declaration.
@@ -2676,8 +2717,16 @@
   for (TemplateParameterList::iterator NewParam = NewParams->begin(),
                                     NewParamEnd = NewParams->end();
        NewParam != NewParamEnd; ++NewParam) {
-    // Variables used to diagnose redundant default arguments
+    // Variables used to diagnose redundant default arguments in the same
+    // translation unit.
     bool RedundantDefaultArg = false;
+    // Variables used to diagnose inconsitent default arguments in different
+    // translation unit.
+    bool InconsistentDefaultArg = false;
+    // Variables used to diagnose the module name of the old param in case the
+    // default argument of OldParam is inconsistent with NewParam.
+    std::string PrevModuleName;
+
     SourceLocation OldDefaultLoc;
     SourceLocation NewDefaultLoc;
 
@@ -2710,7 +2759,13 @@
         OldDefaultLoc = OldTypeParm->getDefaultArgumentLoc();
         NewDefaultLoc = NewTypeParm->getDefaultArgumentLoc();
         SawDefaultArgument = true;
-        RedundantDefaultArg = true;
+        if (!OldTypeParm->getImportedOwningModule())
+          RedundantDefaultArg = true;
+        else if (!hasSameDefaultTemplateArgument(OldTypeParm, NewTypeParm)) {
+          InconsistentDefaultArg = true;
+          PrevModuleName =
+              OldTypeParm->getImportedOwningModule()->getFullModuleName();
+        }
         PreviousDefaultArgLoc = NewDefaultLoc;
       } else if (OldTypeParm && OldTypeParm->hasDefaultArgument()) {
         // Merge the default argument from the old declaration to the
@@ -2755,7 +2810,14 @@
         OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc();
         NewDefaultLoc = NewNonTypeParm->getDefaultArgumentLoc();
         SawDefaultArgument = true;
-        RedundantDefaultArg = true;
+        if (!OldNonTypeParm->getImportedOwningModule())
+          RedundantDefaultArg = true;
+        else if (!hasSameDefaultTemplateArgument(OldNonTypeParm,
+                                                 NewNonTypeParm)) {
+          InconsistentDefaultArg = true;
+          PrevModuleName =
+              OldNonTypeParm->getImportedOwningModule()->getFullModuleName();
+        }
         PreviousDefaultArgLoc = NewDefaultLoc;
       } else if (OldNonTypeParm && OldNonTypeParm->hasDefaultArgument()) {
         // Merge the default argument from the old declaration to the
@@ -2799,7 +2861,14 @@
         OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation();
         NewDefaultLoc = NewTemplateParm->getDefaultArgument().getLocation();
         SawDefaultArgument = true;
-        RedundantDefaultArg = true;
+        if (!OldTemplateParm->getImportedOwningModule())
+          RedundantDefaultArg = true;
+        else if (!hasSameDefaultTemplateArgument(OldTemplateParm,
+                                                 NewTemplateParm)) {
+          InconsistentDefaultArg = true;
+          PrevModuleName =
+              OldTemplateParm->getImportedOwningModule()->getFullModuleName();
+        }
         PreviousDefaultArgLoc = NewDefaultLoc;
       } else if (OldTemplateParm && OldTemplateParm->hasDefaultArgument()) {
         // Merge the default argument from the old declaration to the
@@ -2826,13 +2895,32 @@
       Invalid = true;
     }
 
+    // [basic.def.odr]/13:
+    //     There can be more than one definition of a
+    //     ...
+    //     default template argument
+    //     ...
+    //     in a program provided that each definition appears in a different
+    //     translation unit and the definitions satisfy the [same-meaning
+    //     criteria of the ODR].
+    //
+    // Simply, the design of modules allows the definition of template default
+    // argument to be repeated across translation unit. Note that the ODR is
+    // checked elsewhere. But it is still not allowed to repeat template default
+    // argument in the same translation unit.
     if (RedundantDefaultArg) {
-      // C++ [temp.param]p12:
-      //   A template-parameter shall not be given default arguments
-      //   by two different declarations in the same scope.
       Diag(NewDefaultLoc, diag::err_template_param_default_arg_redefinition);
       Diag(OldDefaultLoc, diag::note_template_param_prev_default_arg);
       Invalid = true;
+    } else if (InconsistentDefaultArg) {
+      // We could only diagnose about the case that the OldParam is imported.
+      // The case NewParam is imported should be handled in ASTReader.
+      Diag(NewDefaultLoc,
+           diag::err_template_param_default_arg_inconsistent_redefinition);
+      Diag(OldDefaultLoc,
+           diag::note_template_param_prev_default_arg_in_other_module)
+          << PrevModuleName;
+      Invalid = true;
     } else if (MissingDefaultArg && TPC != TPC_FunctionTemplate) {
       // C++ [temp.param]p11:
       //   If a template-parameter of a class template has a default
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4771,8 +4771,12 @@
   DefaultIgnore, InGroup<CXXPre17Compat>;
 def err_template_param_default_arg_redefinition : Error<
   "template parameter redefines default argument">;
+def err_template_param_default_arg_inconsistent_redefinition : Error<
+  "template parameter default argument is inconsistent with previous definition">;
 def note_template_param_prev_default_arg : Note<
   "previous default template argument defined here">;
+def note_template_param_prev_default_arg_in_other_module : Note<
+  "previous default template argument defined in module %0">;
 def err_template_param_default_arg_missing : Error<
   "template parameter missing a default argument">;
 def ext_template_parameter_default_in_function_template : ExtWarn<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to