[clang] 354a597 - [C++20] [Modules] Don't complain about duplicated default template argument across modules
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 7 20:13:55 PDT 2022
Author: Chuanqi Xu
Date: 2022-07-08T11:10:51+08:00
New Revision: 354a597b9f3aad2a6a37518d47351adb99688cd2
URL: https://github.com/llvm/llvm-project/commit/354a597b9f3aad2a6a37518d47351adb99688cd2
DIFF: https://github.com/llvm/llvm-project/commit/354a597b9f3aad2a6a37518d47351adb99688cd2.diff
LOG: [C++20] [Modules] Don't complain about duplicated default template argument across modules
See https://github.com/cplusplus/draft/pull/5204 for a detailed
background.
Simply, the test redundant-template-default-arg.cpp attached to this
patch should be accepted instead of being complained about the
redefinition.
Reviewed By: urnathan, rsmith, ChuanqiXu
Differential Revision: https://reviews.llvm.org/D118034
Added:
clang/test/Modules/redundant-template-default-arg.cpp
clang/test/Modules/redundant-template-default-arg2.cpp
clang/test/Modules/redundant-template-default-arg3.cpp
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaModule.cpp
clang/lib/Sema/SemaTemplate.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9e589c70a86d7..c811cd855d503 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4830,8 +4830,12 @@ def warn_cxx14_compat_template_nontype_parm_auto_type : Warning<
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<
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index eddab059a17f5..4fc53a0697a5c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2272,6 +2272,9 @@ class Sema final {
bool isAcceptableSlow(const NamedDecl *D, AcceptableKind Kind);
+ // Determine whether the module M belongs to the current TU.
+ bool isModuleUnitOfCurrentTU(const Module *M) const;
+
public:
/// Get the module unit whose scope we are currently within.
Module *getCurrentModule() const {
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 3aa124d457b03..e9a1ac17ce86e 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -935,3 +935,16 @@ void Sema::PopGlobalModuleFragment() {
"left the wrong module scope, which is not global module fragment");
ModuleScopes.pop_back();
}
+
+bool Sema::isModuleUnitOfCurrentTU(const Module *M) const {
+ assert(M);
+
+ Module *CurrentModuleUnit = getCurrentModule();
+
+ // If we are not in a module currently, M must not be the module unit of
+ // current TU.
+ if (!CurrentModuleUnit)
+ return false;
+
+ return M->isSubModuleOf(CurrentModuleUnit->getTopLevelModule());
+}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index dbfe6164bda2c..8e6a810d95df0 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2643,6 +2643,46 @@ static bool DiagnoseUnexpandedParameterPacks(Sema &S,
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();
+ llvm::FoldingSetNodeID XID, YID;
+ DefaultArgumentX->Profile(XID, C, /*Canonical=*/true);
+ DefaultArgumentY->Profile(YID, C, /*Canonical=*/true);
+ return XID == YID;
+ }
+
+ 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.
@@ -2695,8 +2735,15 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
for (TemplateParameterList::iterator NewParam = NewParams->begin(),
NewParamEnd = NewParams->end();
NewParam != NewParamEnd; ++NewParam) {
- // Variables used to diagnose redundant default arguments
+ // Whether we've seen a duplicate default argument in the same translation
+ // unit.
bool RedundantDefaultArg = false;
+ // Whether we've found inconsis inconsitent default arguments in
diff erent
+ // translation unit.
+ bool InconsistentDefaultArg = false;
+ // The name of the module which contains the inconsistent default argument.
+ std::string PrevModuleName;
+
SourceLocation OldDefaultLoc;
SourceLocation NewDefaultLoc;
@@ -2729,7 +2776,15 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
OldDefaultLoc = OldTypeParm->getDefaultArgumentLoc();
NewDefaultLoc = NewTypeParm->getDefaultArgumentLoc();
SawDefaultArgument = true;
- RedundantDefaultArg = true;
+
+ if (!OldTypeParm->getOwningModule() ||
+ isModuleUnitOfCurrentTU(OldTypeParm->getOwningModule()))
+ 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
@@ -2774,7 +2829,15 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc();
NewDefaultLoc = NewNonTypeParm->getDefaultArgumentLoc();
SawDefaultArgument = true;
- RedundantDefaultArg = true;
+ if (!OldNonTypeParm->getOwningModule() ||
+ isModuleUnitOfCurrentTU(OldNonTypeParm->getOwningModule()))
+ 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
@@ -2818,7 +2881,15 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation();
NewDefaultLoc = NewTemplateParm->getDefaultArgument().getLocation();
SawDefaultArgument = true;
- RedundantDefaultArg = true;
+ if (!OldTemplateParm->getOwningModule() ||
+ isModuleUnitOfCurrentTU(OldTemplateParm->getOwningModule()))
+ 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
@@ -2845,13 +2916,32 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
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
diff erent
+ // 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
diff erent 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
diff --git a/clang/test/Modules/redundant-template-default-arg.cpp b/clang/test/Modules/redundant-template-default-arg.cpp
new file mode 100644
index 0000000000000..6807b45e51395
--- /dev/null
+++ b/clang/test/Modules/redundant-template-default-arg.cpp
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/foo.cppm -I%t -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1 -fprebuilt-module-path=%t -std=c++20 %t/use.cpp -I%t -fsyntax-only -verify
+
+//--- foo.h
+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;
+
+//--- foo.cppm
+module;
+#include "foo.h"
+export module foo;
+
+
+//--- use.cpp
+// expected-no-diagnostics
+import foo;
+#include "foo.h"
diff --git a/clang/test/Modules/redundant-template-default-arg2.cpp b/clang/test/Modules/redundant-template-default-arg2.cpp
new file mode 100644
index 0000000000000..41deb112cfa6e
--- /dev/null
+++ b/clang/test/Modules/redundant-template-default-arg2.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/foo.cppm -I%t -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1 -fprebuilt-module-path=%t -std=c++20 %t/use.cpp -fsyntax-only -verify
+
+//--- foo.cppm
+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;
+
+//--- use.cpp
+import foo;
+template <typename T = int>
+T v; // expected-error {{declaration of 'v' in the global module follows declaration in module foo}}
+ // expected-note at 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 at foo.cppm:6 {{previous declaration is here}}
+
+template <typename T>
+class my_array {}; // expected-error {{redefinition of 'my_array'}}
+ // expected-note at 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 at foo.cppm:12 {{previous declaration is here}}
diff --git a/clang/test/Modules/redundant-template-default-arg3.cpp b/clang/test/Modules/redundant-template-default-arg3.cpp
new file mode 100644
index 0000000000000..8bb222ac91ffc
--- /dev/null
+++ b/clang/test/Modules/redundant-template-default-arg3.cpp
@@ -0,0 +1,113 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/foo.cppm -I%t -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1 -fprebuilt-module-path=%t -std=c++20 %t/use.cpp -I%t/. -fsyntax-only -verify
+
+//--- foo.h
+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;
+
+//--- foo_bad.h
+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;
+
+//--- foo.cppm
+module;
+#include "foo.h"
+export module foo;
+
+//--- use.cpp
+import foo;
+#include "foo_bad.h"
+
+// expected-error at foo_bad.h:1 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:1 {{previous default template argument defined in module foo.<global>}}
+// expected-error at foo_bad.h:4 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:4 {{previous default template argument defined in module foo.<global>}}
+// expected-error at foo_bad.h:10 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:10 {{previous default template argument defined in module foo.<global>}}
+// expected-error at foo_bad.h:17 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:13 {{previous default template argument defined in module foo.<global>}}
+// expected-error at foo_bad.h:23 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:16 {{previous default template argument defined in module foo.<global>}}
+// expected-error at foo_bad.h:27 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:20 {{previous default template argument defined in module foo.<global>}}
+// expected-error at foo_bad.h:31 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:24 {{previous default template argument defined in module foo.<global>}}
+// expected-error at foo_bad.h:34 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:27 {{previous default template argument defined in module foo.<global>}}
+// expected-error at foo_bad.h:40 {{template parameter default argument is inconsistent with previous definition}}
+// expected-note at foo.h:33 {{previous default template argument defined in module foo.<global>}}
More information about the cfe-commits
mailing list