https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/146153
This fixes an issue with the ODR checker not using the as-written type of conversion operators. The odr-checker in general should not have to deal with canonical types, as its purpose is to compare same definitions across TUs, and these need to be same as written, with few exceptions. Using canonical types is specially problematic when expressions are involved, as the types which refer to them generally pick an arbitrary representative expression, and this can lead to false mismatches. This patch makes sure that when hashing the names of declarations, if a DeclarationNameInfo is available, its type source info is used, instead of the type contained in the DeclarationName, which otherwise is always canonical. This patch supersedes #144796, as it fixes the problem without weakening the ODR checker. Fixes https://github.com/llvm/llvm-project/issues/143152 >From c5f95e92ddbf60af42f1296bbaaa38e9f61ec29d Mon Sep 17 00:00:00 2001 From: Matheus Izvekov <mizve...@gmail.com> Date: Fri, 27 Jun 2025 17:22:19 -0300 Subject: [PATCH] [clang] odr-checker fix for conversion operators This fixes an issue with the ODR checker not using the as-written type of conversion operators. The odr-checker in general should not have to deal with canonical types, as its purpose is to compare same definitions across TUs, and these need to be same as written, with few exceptions. Using canonical types is specially problematic when expressions are involved, as the types which refer to them generally pick an arbitrary representative expression, and this can lead to false mismatches. This patch makes sure that when hashing the names of declarations, the type source info contained in the DeclarationNameInfo is used instead of the one contained in the DeclarationName, which otherwise is always canonical. Fixes https://github.com/llvm/llvm-project/pull/144796 --- clang/docs/ReleaseNotes.rst | 5 +++-- clang/include/clang/AST/ODRHash.h | 10 ++++++++-- clang/lib/AST/ODRHash.cpp | 29 +++++++++++++++++++---------- clang/test/Modules/odr_hash.cpp | 23 +++++++++++++++++++++++ 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d9847fadc21e5..019f27c8f736f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -656,7 +656,7 @@ Improvements to Clang's diagnostics false positives in exception-heavy code, though only simple patterns are currently recognized. - + Improvements to Clang's time-trace ---------------------------------- @@ -734,7 +734,7 @@ Bug Fixes in This Version - Fixed incorrect token location when emitting diagnostics for tokens expanded from macros. (#GH143216) - Fixed an infinite recursion when checking constexpr destructors. (#GH141789) - Fixed a crash when a malformed using declaration appears in a ``constexpr`` function. (#GH144264) -- Fixed a bug when use unicode character name in macro concatenation. (#GH145240) +- Fixed a bug when use unicode character name in macro concatenation. (#GH145240) Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -892,6 +892,7 @@ Bug Fixes to AST Handling - Fixed a malformed printout of ``CXXParenListInitExpr`` in certain contexts. - Fixed a malformed printout of certain calling convention function attributes. (#GH143160) - Fixed dependency calculation for TypedefTypes (#GH89774) +- The ODR checker now correctly hashes the names of conversion operators. (#GH143152) - Fixed the right parenthesis source location of ``CXXTemporaryObjectExpr``. (#GH143711) Miscellaneous Bug Fixes diff --git a/clang/include/clang/AST/ODRHash.h b/clang/include/clang/AST/ODRHash.h index a923901b32dc0..11f917a1a4634 100644 --- a/clang/include/clang/AST/ODRHash.h +++ b/clang/include/clang/AST/ODRHash.h @@ -96,7 +96,13 @@ class ODRHash { void AddNestedNameSpecifier(const NestedNameSpecifier *NNS); void AddDependentTemplateName(const DependentTemplateStorage &Name); void AddTemplateName(TemplateName Name); - void AddDeclarationName(DeclarationName Name, bool TreatAsDecl = false); + void AddDeclarationNameInfo(DeclarationNameInfo NameInfo, + bool TreatAsDecl = false); + void AddDeclarationName(DeclarationName Name, bool TreatAsDecl = false) { + AddDeclarationNameInfo(DeclarationNameInfo(Name, SourceLocation()), + TreatAsDecl); + } + void AddTemplateArgument(TemplateArgument TA); void AddTemplateParameterList(const TemplateParameterList *TPL); @@ -108,7 +114,7 @@ class ODRHash { static bool isSubDeclToBeProcessed(const Decl *D, const DeclContext *Parent); private: - void AddDeclarationNameImpl(DeclarationName Name); + void AddDeclarationNameInfoImpl(DeclarationNameInfo NameInfo); }; } // end namespace clang diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp index f8446dfbc6859..7fdfcfa3014f3 100644 --- a/clang/lib/AST/ODRHash.cpp +++ b/clang/lib/AST/ODRHash.cpp @@ -30,19 +30,21 @@ void ODRHash::AddIdentifierInfo(const IdentifierInfo *II) { ID.AddString(II->getName()); } -void ODRHash::AddDeclarationName(DeclarationName Name, bool TreatAsDecl) { +void ODRHash::AddDeclarationNameInfo(DeclarationNameInfo NameInfo, + bool TreatAsDecl) { if (TreatAsDecl) // Matches the NamedDecl check in AddDecl AddBoolean(true); - AddDeclarationNameImpl(Name); + AddDeclarationNameInfoImpl(NameInfo); if (TreatAsDecl) // Matches the ClassTemplateSpecializationDecl check in AddDecl AddBoolean(false); } -void ODRHash::AddDeclarationNameImpl(DeclarationName Name) { +void ODRHash::AddDeclarationNameInfoImpl(DeclarationNameInfo NameInfo) { + DeclarationName Name = NameInfo.getName(); // Index all DeclarationName and use index numbers to refer to them. auto Result = DeclNameMap.insert(std::make_pair(Name, DeclNameMap.size())); ID.AddInteger(Result.first->second); @@ -85,7 +87,11 @@ void ODRHash::AddDeclarationNameImpl(DeclarationName Name) { } case DeclarationName::CXXConstructorName: case DeclarationName::CXXDestructorName: - AddQualType(Name.getCXXNameType()); + case DeclarationName::CXXConversionFunctionName: + if (auto *TSI = NameInfo.getNamedTypeInfo()) + AddQualType(TSI->getType()); + else + AddQualType(Name.getCXXNameType()); break; case DeclarationName::CXXOperatorName: ID.AddInteger(Name.getCXXOverloadedOperator()); @@ -93,9 +99,6 @@ void ODRHash::AddDeclarationNameImpl(DeclarationName Name) { case DeclarationName::CXXLiteralOperatorName: AddIdentifierInfo(Name.getCXXLiteralIdentifier()); break; - case DeclarationName::CXXConversionFunctionName: - AddQualType(Name.getCXXNameType()); - break; case DeclarationName::CXXUsingDirective: break; case DeclarationName::CXXDeductionGuideName: { @@ -314,7 +317,10 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> { } void VisitNamedDecl(const NamedDecl *D) { - Hash.AddDeclarationName(D->getDeclName()); + if (const auto *FD = dyn_cast<FunctionDecl>(D)) + Hash.AddDeclarationNameInfo(FD->getNameInfo()); + else + Hash.AddDeclarationName(D->getDeclName()); Inherited::VisitNamedDecl(D); } @@ -828,7 +834,10 @@ void ODRHash::AddDecl(const Decl *D) { return; } - AddDeclarationName(ND->getDeclName()); + if (auto *FD = dyn_cast<FunctionDecl>(D)) + AddDeclarationNameInfo(FD->getNameInfo()); + 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 @@ -1017,7 +1026,7 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> { } void VisitDecltypeType(const DecltypeType *T) { - AddStmt(T->getUnderlyingExpr()); + Hash.AddStmt(T->getUnderlyingExpr()); VisitType(T); } 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
