[clang] odr-checker fix for conversion operators (#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 #143152

Author: mizvekov

clang/docs/ReleaseNotes.rst

@@ -901,6 +901,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

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

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,17 +87,18 @@ 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());
     break;
   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);
   }
 

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