https://github.com/emaxx-google updated https://github.com/llvm/llvm-project/pull/144796
>From 2fd016fd5d0bbc8d9922e7dbdc3bf29fd9099450 Mon Sep 17 00:00:00 2001 From: Maksim Ivanov <em...@google.com> Date: Wed, 18 Jun 2025 23:13:55 +0200 Subject: [PATCH 1/2] [clang] ODR hashes depth+index and not name of TemplateTypeParm Change the ODR hashing logic to use the depth+index indices instead of template parameter names. This prevents spurious ODR errors in header module builds when the type canonicalization picks up different expressions - with different template parameter names (profiling used for canonicalization ignores template parameter names). This fixes #143152. This commit removes test assertions from the regression test of https://github.com/llvm/llvm-project/pull/108866 - the compiler errors asserted by that test stop firing with this commit. It's questionable whether these diagnostics were correct. We add a FIXME to find a better regression test for that fix. --- clang/lib/AST/ODRHash.cpp | 18 +++++++++++++++++- clang/test/Modules/odr_hash.cpp | 23 +++++++++++++++++++++++ clang/test/PCH/race-condition.cpp | 11 ++++++----- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp index f8446dfbc6859..84fac90a03e87 100644 --- a/clang/lib/AST/ODRHash.cpp +++ b/clang/lib/AST/ODRHash.cpp @@ -828,7 +828,23 @@ void ODRHash::AddDecl(const Decl *D) { return; } - AddDeclarationName(ND->getDeclName()); + // For template parameters, use depth+index instead of name, because type + // canonicalization can change the name of the template parameter. + if (auto *TTPD = dyn_cast<TemplateTypeParmDecl>(ND)) { + ID.AddInteger(TTPD->getDepth()); + ID.AddInteger(TTPD->getIndex()); + AddBoolean(TTPD->isParameterPack()); + } else if (auto *NTTPD = dyn_cast<NonTypeTemplateParmDecl>(ND)) { + ID.AddInteger(NTTPD->getDepth()); + ID.AddInteger(NTTPD->getIndex()); + AddBoolean(NTTPD->isParameterPack()); + } else if (auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(ND)) { + ID.AddInteger(TTPD->getDepth()); + ID.AddInteger(TTPD->getIndex()); + AddBoolean(TTPD->isParameterPack()); + } else { + AddDeclarationName(ND->getDeclName()); + } // If this was a specialization we should take into account its template // arguments. This helps to reduce collisions coming when visiting template diff --git a/clang/test/Modules/odr_hash.cpp b/clang/test/Modules/odr_hash.cpp index da24b1fe8729a..8ef53e32f2e95 100644 --- a/clang/test/Modules/odr_hash.cpp +++ b/clang/test/Modules/odr_hash.cpp @@ -5164,6 +5164,29 @@ namespace A { A::X x; #endif +namespace TemplateDecltypeOperator { + +#if defined(FIRST) || defined(SECOND) +template <class T6> +T6 func(); +#endif + +#if defined(SECOND) +template <class UnrelatedT> +using UnrelatedAlias = decltype(func<UnrelatedT>())(); +#endif + +#if defined(FIRST) || defined(SECOND) +class A { + template <class T6> + operator decltype(func<T6>()) () {} +}; +#else +A a; +#endif + +} + // Keep macros contained to one file. #ifdef FIRST #undef FIRST diff --git a/clang/test/PCH/race-condition.cpp b/clang/test/PCH/race-condition.cpp index 752b0cc3ff628..666d92ed2ce8b 100644 --- a/clang/test/PCH/race-condition.cpp +++ b/clang/test/PCH/race-condition.cpp @@ -31,11 +31,12 @@ constexpr enable_if_t<meta<F>::value == 2, void> midpoint(F) {} #else -// expected-error@27{{'N::midpoint' has different definitions in different modules; defined here first difference is 1st parameter with type 'F'}} -// expected-error@24{{'N::midpoint' has different definitions in different modules; defined here first difference is 1st parameter with type 'U'}} -// expected-note@21{{but in '' found 1st parameter with type 'T'}} +// FIXME: Change the test to trigger a suitable error: previously the test +// asserted the ODR error ("'N::midpoint' has different definitions in different +// modules"), which isn't fully correct as there's only one module, and since a +// change in the ODR hash calculation this error isn't triggered anymore. + int x = N::something; -// expected-error@37{{no member named 'something' in namespace 'N'}} -// expected-note@21{{but in '' found 1st parameter with type 'T'}} +// expected-error@38{{no member named 'something' in namespace 'N'}} #endif >From 4f800c8f749489c221d4eb710c4bd235e72cd20f Mon Sep 17 00:00:00 2001 From: Maksim Ivanov <em...@google.com> Date: Thu, 19 Jun 2025 00:10:41 +0200 Subject: [PATCH 2/2] fix assert line number --- clang/test/PCH/race-condition.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/PCH/race-condition.cpp b/clang/test/PCH/race-condition.cpp index 666d92ed2ce8b..fdb4c40c6275c 100644 --- a/clang/test/PCH/race-condition.cpp +++ b/clang/test/PCH/race-condition.cpp @@ -37,6 +37,6 @@ constexpr enable_if_t<meta<F>::value == 2, void> midpoint(F) {} // change in the ODR hash calculation this error isn't triggered anymore. int x = N::something; -// expected-error@38{{no member named 'something' in namespace 'N'}} +// expected-error@39{{no member named 'something' in namespace 'N'}} #endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
