[Modules] Incorrect ODR detection for unresolved using type

[ChuanqiXu]

Summary

See https://github.com/llvm/llvm-project/issues/52713.
This patch is a attempt to fix bug52713.

The key reason for the bug is that isSameEntity couldn't recognize void bar(Ty) in imported module and current module as the same entity. Since the compiler thinks the Ty type in the argument in imported module and the current are different type.

I found the reason is that the isSameEntity would judge if two types are the same by comparing the addressed of their canonical type. But the Ty type in this example is UnresolvedUsingType and the canonical type of UnresolvedUsingType is always null. So that the compare result would be different always.

My solution in this patch is to not create a new UnresolvedUsingType when deserialization if there is already a canonical type in canonical declaration. I am not sure if this is proper but it runs well currently.

[rjmccall]

This seems reasonable, but I think it's probably better to just doing it in the ASTContext code (getTypeDeclTypeSlow) the same way it's done for e.g. RecordDecls with previous declarations.

[ChuanqiXu]

Address comments.

[rjmccall]

LGTM, thanks.

clang/include/clang/AST/ASTContext.h

public:
   /// typedef-name decl.
   QualType getTypedefType(const TypedefNameDecl *Decl,
                           QualType Underlying = QualType()) const;
 
   QualType getRecordType(const RecordDecl *Decl) const;
 
   QualType getEnumType(const EnumDecl *Decl) const;
 
+  QualType
+  getUnresolvedUsingType(const UnresolvedUsingTypenameDecl *Decl) const;
+
   QualType getInjectedClassNameType(CXXRecordDecl *Decl, QualType TST) const;
 
   QualType getAttributedType(attr::Kind attrKind,
                              QualType modifiedType,
                              QualType equivalentType);
 
   QualType getSubstTemplateTypeParmType(const TemplateTypeParmType *Replaced,
                                         QualType Replacement) const;

clang/include/clang/AST/TypeProperties.td

 }
 
 let Class = UnresolvedUsingType in {
   def : Property<"declaration", DeclRef> {
     let Read = [{ node->getDecl() }];
   }
 
   def : Creator<[{
-    return ctx.getTypeDeclType(cast<UnresolvedUsingTypenameDecl>(declaration));
+    return ctx.getUnresolvedUsingType(cast<UnresolvedUsingTypenameDecl>(declaration));
   }]>;
 }
 
 let Class = TypedefType in {
   def : Property<"declaration", DeclRef> {
     let Read = [{ node->getDecl() }];
   }
   def : Property<"canonicalType", Optional<QualType>> {

clang/lib/AST/ASTContext.cpp

QualType ASTContext::getTypeDeclTypeSlow(const TypeDecl *Decl) const {
   if (const auto *Record = dyn_cast<RecordDecl>(Decl)) {
     assert(Record->isFirstDecl() && "struct/union has previous declaration");
     assert(!NeedsInjectedClassNameType(Record));
     return getRecordType(Record);
   } else if (const auto *Enum = dyn_cast<EnumDecl>(Decl)) {
     assert(Enum->isFirstDecl() && "enum has previous declaration");
     return getEnumType(Enum);
   } else if (const auto *Using = dyn_cast<UnresolvedUsingTypenameDecl>(Decl)) {
-    Type *newType = new (*this, TypeAlignment) UnresolvedUsingType(Using);
-    Decl->TypeForDecl = newType;
-    Types.push_back(newType);
+    return getUnresolvedUsingType(Using);
   } else
     llvm_unreachable("TypeDecl without a type?");
 
   return QualType(Decl->TypeForDecl, 0);
 }
 
 /// getTypedefType - Return the unique reference to the type for the
 /// specified typedef name decl.
if (PrevDecl->TypeForDecl)
       return QualType(Decl->TypeForDecl = PrevDecl->TypeForDecl, 0);
 
   auto *newType = new (*this, TypeAlignment) EnumType(Decl);
   Decl->TypeForDecl = newType;
   Types.push_back(newType);
   return QualType(newType, 0);
 }
 
+QualType ASTContext::getUnresolvedUsingType(
+    const UnresolvedUsingTypenameDecl *Decl) const {
+  if (Decl->TypeForDecl)
+    return QualType(Decl->TypeForDecl, 0);
+
+  if (const UnresolvedUsingTypenameDecl *CanonicalDecl =
+          Decl->getCanonicalDecl())
+    if (CanonicalDecl->TypeForDecl)
+      return QualType(Decl->TypeForDecl = CanonicalDecl->TypeForDecl, 0);
+
+  Type *newType = new (*this, TypeAlignment) UnresolvedUsingType(Decl);
+  Decl->TypeForDecl = newType;
+  Types.push_back(newType);
+  return QualType(newType, 0);
+}
+
 QualType ASTContext::getAttributedType(attr::Kind attrKind,
                                        QualType modifiedType,
                                        QualType equivalentType) {
   llvm::FoldingSetNodeID id;
   AttributedType::Profile(id, attrKind, modifiedType, equivalentType);
 
   void *insertPos = nullptr;
   AttributedType *type = AttributedTypes.FindNodeOrInsertPos(id, insertPos);

clang/test/Modules/Inputs/odr_using_dependent_name/X.cppm

+module;
+#include "foo.h"
+export module X;

clang/test/Modules/Inputs/odr_using_dependent_name/foo.h

+template <class T>
+struct bar {
+  using Ty = int;
+};
+template <class T>
+struct foo : public bar<T> {
+  using typename bar<T>::Ty;
+  void baz(Ty);
+};

clang/test/Modules/odr_using_dependent_name.cppm

+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %clang -std=c++20 %S/Inputs/odr_using_dependent_name/X.cppm --precompile -o %t/X.pcm
+// RUN: %clang -std=c++20 -I%S/Inputs/odr_using_dependent_name -fprebuilt-module-path=%t %s --precompile -c
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module Y;
+import X;