llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (Qayyum-Ahmed) <details> <summary>Changes</summary> Fixes #<!-- -->184682 : Sema::CheckTemplateParameterList unconditionally called OldParam->getImportedOwningModule()->getFullModuleName() when diagnosing repeated default template arguments, but for declarations in the global module fragment or a local named module getImportedOwningModule() is null, leading to a null dereference. The fix avoids the crash by only calling Module::getFullModuleName() when the previous declaration comes from an imported module (i.e. getImportedOwningModule() is non-null). For declarations in the current TU, we now distinguish module-map modules (Clang header modules) from C++20 named modules and the global module fragment: in module-map modules we allow repeated identical defaults, while in named modules and the global fragment we still diagnose redefinitions. --- Full diff: https://github.com/llvm/llvm-project/pull/185237.diff 2 Files Affected: - (modified) clang/lib/Sema/SemaTemplate.cpp (+48-22) - (added) clang/test/CXX/module/basic/basic.def.odr/p7.cppm (+63) ``````````diff diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 9b0bec20618a0..e713011ca80e4 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -26,6 +26,7 @@ #include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/PartialDiagnostic.h" +#include "clang/Basic/Module.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/TargetInfo.h" #include "clang/Sema/DeclSpec.h" @@ -2472,13 +2473,23 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams, NewDefaultLoc = NewTypeParm->getDefaultArgumentLoc(); SawDefaultArgument = true; - if (!OldTypeParm->getOwningModule()) + bool SameDefault = + getASTContext().isSameDefaultTemplateArgument(OldTypeParm, + NewTypeParm); + if (Module *ImportedM = OldTypeParm->getImportedOwningModule()) { + if (!SameDefault) { + InconsistentDefaultArg = true; + PrevModuleName = ImportedM->getFullModuleName(); + } + } else if (Module *LocalM = OldTypeParm->getLocalOwningModule()) { + if (LocalM->isModuleMapModule()) { + if (!SameDefault) + RedundantDefaultArg = true; + } else { + RedundantDefaultArg = true; + } + } else { RedundantDefaultArg = true; - else if (!getASTContext().isSameDefaultTemplateArgument(OldTypeParm, - NewTypeParm)) { - InconsistentDefaultArg = true; - PrevModuleName = - OldTypeParm->getImportedOwningModule()->getFullModuleName(); } PreviousDefaultArgLoc = NewDefaultLoc; } else if (OldTypeParm && OldTypeParm->hasDefaultArgument()) { @@ -2527,13 +2538,22 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams, OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc(); NewDefaultLoc = NewNonTypeParm->getDefaultArgumentLoc(); SawDefaultArgument = true; - if (!OldNonTypeParm->getOwningModule()) + bool SameDefault = getASTContext().isSameDefaultTemplateArgument( + OldNonTypeParm, NewNonTypeParm); + if (Module *ImportedM = OldNonTypeParm->getImportedOwningModule()) { + if (!SameDefault) { + InconsistentDefaultArg = true; + PrevModuleName = ImportedM->getFullModuleName(); + } + } else if (Module *LocalM = OldNonTypeParm->getLocalOwningModule()) { + if (LocalM->isModuleMapModule()) { + if (!SameDefault) + RedundantDefaultArg = true; + } else { + RedundantDefaultArg = true; + } + } else { RedundantDefaultArg = true; - else if (!getASTContext().isSameDefaultTemplateArgument( - OldNonTypeParm, NewNonTypeParm)) { - InconsistentDefaultArg = true; - PrevModuleName = - OldNonTypeParm->getImportedOwningModule()->getFullModuleName(); } PreviousDefaultArgLoc = NewDefaultLoc; } else if (OldNonTypeParm && OldNonTypeParm->hasDefaultArgument()) { @@ -2578,13 +2598,22 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams, OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation(); NewDefaultLoc = NewTemplateParm->getDefaultArgument().getLocation(); SawDefaultArgument = true; - if (!OldTemplateParm->getOwningModule()) + bool SameDefault = getASTContext().isSameDefaultTemplateArgument( + OldTemplateParm, NewTemplateParm); + if (Module *ImportedM = OldTemplateParm->getImportedOwningModule()) { + if (!SameDefault) { + InconsistentDefaultArg = true; + PrevModuleName = ImportedM->getFullModuleName(); + } + } else if (Module *LocalM = OldTemplateParm->getLocalOwningModule()) { + if (LocalM->isModuleMapModule()) { + if (!SameDefault) + RedundantDefaultArg = true; + } else { + RedundantDefaultArg = true; + } + } else { RedundantDefaultArg = true; - else if (!getASTContext().isSameDefaultTemplateArgument( - OldTemplateParm, NewTemplateParm)) { - InconsistentDefaultArg = true; - PrevModuleName = - OldTemplateParm->getImportedOwningModule()->getFullModuleName(); } PreviousDefaultArgLoc = NewDefaultLoc; } else if (OldTemplateParm && OldTemplateParm->hasDefaultArgument()) { @@ -8138,9 +8167,6 @@ static Expr *BuildExpressionFromNonTypeTemplateArgumentValue( return MakeInitList(Elts); } - case APValue::Matrix: - llvm_unreachable("Matrix template argument expression not yet supported"); - case APValue::None: case APValue::Indeterminate: llvm_unreachable("Unexpected APValue kind."); @@ -11841,4 +11867,4 @@ SourceLocation Sema::getTopMostPointOfInstantiation(const NamedDecl *N) const { return CSC.PointOfInstantiation; } return N->getLocation(); -} +} \ No newline at end of file diff --git a/clang/test/CXX/module/basic/basic.def.odr/p7.cppm b/clang/test/CXX/module/basic/basic.def.odr/p7.cppm new file mode 100644 index 0000000000000..89701b834f081 --- /dev/null +++ b/clang/test/CXX/module/basic/basic.def.odr/p7.cppm @@ -0,0 +1,63 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s + +// Global module fragment for named module 'a'. +module; + +// Case 1: original crash – type parameter in GMF +template <class = int> class FooGMF; // expected-note {{previous default template argument defined here}} +template <class = void> class FooGMF; // expected-error {{template parameter redefines default argument}} + +// Case 2: same-TU redefinition without explicit template name +template<class T = int> class FooNonMod; // expected-note {{previous default template argument defined here}} +template<class T = void> class FooNonMod; // expected-error {{template parameter redefines default argument}} + +// Case 3: non-type parameter in GMF +template <int N = 5> class NTGMF; // expected-note {{previous default template argument defined here}} +template <int N = 7> class NTGMF; // expected-error {{template parameter redefines default argument}} + + +// Case 4: legal vs illegal redefinitions across declaration/definition +template <class T = int> class Legal; +template <class T> class Legal { T value; }; + +template <class T = int> class Illegal; // expected-note {{previous default template argument defined here}} +template <class T = void> class Illegal { T value; }; // expected-error {{template parameter redefines default argument}} + +// Case 5: multiple redeclarations (type and non-type) +// When the 3rd decl fires, Clang notes every prior decl that had a default. +template <class = int> class Multi; // expected-note {{previous default template argument defined here}} +template <class = int> class Multi; // expected-error {{template parameter redefines default argument}} expected-note {{previous default template argument defined here}} +template <class = float> class Multi; // expected-error {{template parameter redefines default argument}} + +template <int N = 1> class MultiNT; // expected-note {{previous default template argument defined here}} +template <int N = 1> class MultiNT; // expected-error {{template parameter redefines default argument}} expected-note {{previous default template argument defined here}} +template <int N = 2> class MultiNT; // expected-error {{template parameter redefines default argument}} + +// Case 6: multiple parameters +// For multi-param templates, each parameter gets its own error+note. +// For a 3-decl chain, the 3rd decl notes all prior decls that had defaults (per param). +template <class T = int, class U = double> class Pair; // expected-note 2 {{previous default template argument defined here}} +template <class T = int, class U = double> class Pair; // expected-error 2 {{template parameter redefines default argument}} expected-note 2 {{previous default template argument defined here}} +template <class T = void, class U = float> class Pair; // expected-error 2 {{template parameter redefines default argument}} + +template <int N = 5, char C = 'a'> class NTMulti; // expected-note 2 {{previous default template argument defined here}} +template <int N = 5, char C = 'a'> class NTMulti; // expected-error 2 {{template parameter redefines default argument}} expected-note 2 {{previous default template argument defined here}} +template <int N = 7, char C = 'b'> class NTMulti; // expected-error 2 {{template parameter redefines default argument}} + +// Case 7: template-template parameter defaults +// Same cascading note pattern for a 3-decl chain with 2 params. +template <class> class A; +template <class> class B; + +template <template <class> class TT = A, class T = int> class TmplT; // expected-note 2 {{previous default template argument defined here}} +template <template <class> class TT = A, class T = int> class TmplT; // expected-error 2 {{template parameter redefines default argument}} expected-note 2 {{previous default template argument defined here}} +template <template <class> class TT = B, class T = void> class TmplT; // expected-error 2 {{template parameter redefines default argument}} + +// Case 8: forward declaration pattern +template <class T = int> class Forward; // expected-note {{previous default template argument defined here}} +template <class T> class Forward { T value; }; +template <class T = void> class Forward; // expected-error {{template parameter redefines default argument}} + +export module a; + +int main() {} // expected-warning {{'main' never has module linkage}} \ No newline at end of file `````````` </details> https://github.com/llvm/llvm-project/pull/185237 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
