llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Maksim Ivanov (emaxx-google) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/144796.diff 3 Files Affected: - (modified) clang/lib/AST/ODRHash.cpp (+17-1) - (modified) clang/test/Modules/odr_hash.cpp (+23) - (modified) clang/test/PCH/race-condition.cpp (+6-5) ``````````diff 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 `````````` </details> https://github.com/llvm/llvm-project/pull/144796 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
