[ODRHash] Hash type-as-written

Close llvm#63947
Close llvm#63595

This is suggested by @rsmith in
https://reviews.llvm.org/D154324#inline-1508868

Reviewed By: rsmith

Differential Revision: https://reviews.llvm.org/D156210

clang/lib/AST/ODRHash.cpp

@@ -295,9 +295,9 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
   }
 
   void VisitValueDecl(const ValueDecl *D) {
-    if (!isa<FunctionDecl>(D)) {
-      AddQualType(D->getType());
-    }
+    if (auto *DD = dyn_cast<DeclaratorDecl>(D); DD && DD->getTypeSourceInfo())
+      AddQualType(DD->getTypeSourceInfo()->getType());
+
     Inherited::VisitValueDecl(D);
   }
 
@@ -351,7 +351,7 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
   void VisitObjCPropertyDecl(const ObjCPropertyDecl *D) {
     ID.AddInteger(D->getPropertyAttributes());
     ID.AddInteger(D->getPropertyImplementation());
-    AddQualType(D->getType());
+    AddQualType(D->getTypeSourceInfo()->getType());
     AddDecl(D);
 
     Inherited::VisitObjCPropertyDecl(D);
@@ -394,7 +394,9 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
 
     AddDecl(Method);
 
-    AddQualType(Method->getReturnType());
+    if (Method->getReturnTypeSourceInfo())
+      AddQualType(Method->getReturnTypeSourceInfo()->getType());
+
     ID.AddInteger(Method->param_size());
     for (auto Param : Method->parameters())
       Hash.AddSubDecl(Param);
@@ -593,7 +595,7 @@ void ODRHash::AddCXXRecordDecl(const CXXRecordDecl *Record) {
   ID.AddInteger(Record->getNumBases());
   auto Bases = Record->bases();
   for (const auto &Base : Bases) {
-    AddQualType(Base.getType());
+    AddQualType(Base.getTypeSourceInfo()->getType());
     ID.AddInteger(Base.isVirtual());
     ID.AddInteger(Base.getAccessSpecifierAsWritten());
   }
@@ -912,29 +914,7 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
   void VisitType(const Type *T) {}
 
   void VisitAdjustedType(const AdjustedType *T) {
-    QualType Original = T->getOriginalType();
-    QualType Adjusted = T->getAdjustedType();
-
-    // The original type and pointee type can be the same, as in the case of
-    // function pointers decaying to themselves.  Set a bool and only process
-    // the type once, to prevent doubling the work.
-    SplitQualType split = Adjusted.split();
-    if (auto Pointer = dyn_cast<PointerType>(split.Ty)) {
-      if (Pointer->getPointeeType() == Original) {
-        Hash.AddBoolean(true);
-        ID.AddInteger(split.Quals.getAsOpaqueValue());
-        AddQualType(Original);
-        VisitType(T);
-        return;
-      }
-    }
-
-    // The original type and pointee type are different, such as in the case
-    // of a array decaying to an element pointer.  Set a bool to false and
-    // process both types.
-    Hash.AddBoolean(false);
-    AddQualType(Original);
-    AddQualType(Adjusted);
+    AddQualType(T->getOriginalType());
 
     VisitType(T);
   }
@@ -973,7 +953,6 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
   void VisitAttributedType(const AttributedType *T) {
     ID.AddInteger(T->getAttrKind());
     AddQualType(T->getModifiedType());
-    AddQualType(T->getEquivalentType());
 
     VisitType(T);
   }
@@ -995,7 +974,6 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
 
   void VisitDecltypeType(const DecltypeType *T) {
     AddStmt(T->getUnderlyingExpr());
-    AddQualType(T->getUnderlyingType());
     VisitType(T);
   }
 
@@ -1184,31 +1162,12 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
 
   void VisitTypedefType(const TypedefType *T) {
     AddDecl(T->getDecl());
-    QualType UnderlyingType = T->getDecl()->getUnderlyingType();
-    VisitQualifiers(UnderlyingType.getQualifiers());
-    while (true) {
-      if (const TypedefType *Underlying =
-              dyn_cast<TypedefType>(UnderlyingType.getTypePtr())) {
-        UnderlyingType = Underlying->getDecl()->getUnderlyingType();
-        continue;
-      }
-      if (const ElaboratedType *Underlying =
-              dyn_cast<ElaboratedType>(UnderlyingType.getTypePtr())) {
-        UnderlyingType = Underlying->getNamedType();
-        continue;
-      }
-
-      break;
-    }
-    AddType(UnderlyingType.getTypePtr());
     VisitType(T);
   }
 
   void VisitTypeOfExprType(const TypeOfExprType *T) {
     AddStmt(T->getUnderlyingExpr());
     Hash.AddBoolean(T->isSugared());
-    if (T->isSugared())
-      AddQualType(T->desugar());
 
     VisitType(T);
   }

clang/test/Modules/odr_hash-gnu.cpp

@@ -65,9 +65,10 @@ Invalid1 i1;
 // expected-error@first.h:* {{'Types::TypeOfExpr::Invalid1' has different definitions in different modules; first difference is definition in module 'FirstModule' found field 'x' with type 'typeof (1 + 2)' (aka 'int')}}
 // expected-note@second.h:* {{but in 'SecondModule' found field 'x' with type 'typeof (3)' (aka 'int')}}
 Invalid2 i2;
-// expected-error@second.h:* {{'Types::TypeOfExpr::Invalid2::x' from module 'SecondModule' is not present in definition of 'Types::TypeOfExpr::Invalid2' in module 'FirstModule'}}
-// expected-note@first.h:* {{declaration of 'x' does not match}}
+
 Valid v;
+
+// FIXME: We should diagnose the different definitions of `global`.
 #endif
 }  // namespace TypeOfExpr
 
@@ -113,8 +114,9 @@ Invalid2 i2;
 // expected-error@first.h:* {{'Types::TypeOf::Invalid2' has different definitions in different modules; first difference is definition in module 'FirstModule' found field 'x' with type 'typeof(int)' (aka 'int')}}
 // expected-note@second.h:* {{but in 'SecondModule' found field 'x' with type 'typeof(I)' (aka 'int')}}
 Invalid3 i3;
-// expected-error@second.h:* {{'Types::TypeOf::Invalid3::x' from module 'SecondModule' is not present in definition of 'Types::TypeOf::Invalid3' in module 'FirstModule'}}
-// expected-note@first.h:* {{declaration of 'x' does not match}}
+
+// FIXME: We should reject the `Invalid3` due to the inconsistent definition of `T`.
+
 Valid v;
 #endif
 }  // namespace TypeOf

clang/test/Modules/odr_hash.cpp

@@ -1046,8 +1046,7 @@ struct S3 {
 };
 #else
 S3 s3;
-// expected-error@first.h:* {{'TypeDef::S3::a' from module 'FirstModule' is not present in definition of 'TypeDef::S3' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'a' does not match}}
+// FIXME: We should reject the merge of `S3` due to the inconsistent definition of `T`.
 #endif
 
 #if defined(FIRST)
@@ -1172,8 +1171,7 @@ struct S3 {
 };
 #else
 S3 s3;
-// expected-error@first.h:* {{'Using::S3::a' from module 'FirstModule' is not present in definition of 'Using::S3' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'a' does not match}}
+// FIXME: We should reject the merge of `S3` due to the inconsistent definition of `T`.
 #endif
 
 #if defined(FIRST)
@@ -1361,8 +1359,6 @@ struct S1 {
 };
 #else
 S1 s1;
-// expected-error@first.h:* {{'ElaboratedType::S1::x' from module 'FirstModule' is not present in definition of 'ElaboratedType::S1' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'x' does not match}}
 #endif
 
 #define DECLS \
@@ -3616,8 +3612,8 @@ auto function1 = invalid1;
 // expected-error@second.h:* {{'Types::Decltype::invalid1' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
 // expected-note@first.h:* {{but in 'FirstModule' found a different body}}
 auto function2 = invalid2;
-// expected-error@second.h:* {{'Types::Decltype::invalid2' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
-// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+// FIXME: We should reject the merge of `invalid2` and diagnose about the
+// inconsistent definition of `global`.
 auto function3 = valid;
 #endif
 }  // namespace Decltype
@@ -3700,8 +3696,7 @@ void valid() {
 }
 #else
 auto function1 = invalid1;
-// expected-error@second.h:* {{'Types::DeducedTemplateSpecialization::invalid1' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
-// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+// FIXME: We should reject the merge of `invalid1` due to the inconsistent definition.
 auto function2 = invalid2;
 // expected-error@second.h:* {{'Types::DeducedTemplateSpecialization::invalid2' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
 // expected-note@first.h:* {{but in 'FirstModule' found a different body}}
@@ -4946,8 +4941,42 @@ class S4 {
 #else
 S4 s4;
 #endif
+
+#if defined(FIRST)
+namespace NS5 {
+struct T5;
+} // namespace NS5
+class S5 {
+  NS5::T5* t = 0;
+};
+#elif defined(SECOND)
+namespace NS5 {
+typedef struct T5_Other {} T5;
+} // namespace NS4
+class S5 {
+  NS5::T5* t = 0;
+};
+#else
+S5 s5;
+// expected-error@first.h:* {{'TypedefStruct::S5::t' from module 'FirstModule' is not present in definition of 'TypedefStruct::S5' in module 'SecondModule'}}
+// expected-note@second.h:* {{declaration of 't' does not match}}
+#endif
 } // namespace TypedefStruct
 
+#if defined (FIRST)
+typedef int T;
+namespace A {
+  struct X { T n; };
+}
+#elif defined(SECOND)
+namespace A {
+  typedef int T;
+  struct X { T n; };
+}
+#else
+A::X x;
+#endif
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST

clang/test/Modules/odr_hash.mm

@@ -112,8 +112,7 @@ void valid() {
 // expected-error@second.h:* {{Types::Attributed::invalid1' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
 // expected-note@first.h:* {{but in 'FirstModule' found a different body}}
 auto function2 = invalid2;
-// expected-error@second.h:* {{'Types::Attributed::invalid2' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}}
-// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+
 auto function3 = valid;
 #endif
 }  // namespace Attributed
@@ -266,12 +265,7 @@ @interface Interface6 <T1 : I1 *, T2 : I2 *> {
 }
 @end
 #else
-// expected-error@first.h:* {{'Interface4::x' from module 'FirstModule' is not present in definition of 'Interface4' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'x' does not match}}
-// expected-error@first.h:* {{'Interface5::y' from module 'FirstModule' is not present in definition of 'Interface5' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'y' does not match}}
-// expected-error@first.h:* {{'Interface6::z' from module 'FirstModule' is not present in definition of 'Interface6' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'z' does not match}}
+
 #endif
 
 namespace Types {
@@ -291,14 +285,14 @@ @interface Interface6 <T1 : I1 *, T2 : I2 *> {
 };
 #else
 Invalid1 i1;
-// expected-error@first.h:* {{'Types::ObjCTypeParam::Invalid1::x' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid1' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'x' does not match}}
+
 Invalid2 i2;
-// expected-error@first.h:* {{'Types::ObjCTypeParam::Invalid2::y' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid2' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'y' does not match}}
+
 Invalid3 i3;
-// expected-error@first.h:* {{'Types::ObjCTypeParam::Invalid3::z' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid3' in module 'SecondModule'}}
-// expected-note@second.h:* {{declaration of 'z' does not match}}
+
+// FIXME: We should reject to merge these structs and diagnose for the
+// different definitions for Interface4/Interface5/Interface6.
+
 #endif
 
 }  // namespace ObjCTypeParam

clang/test/Modules/pr63595.cppm

@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module1.cppm -o %t/module1.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module2.cppm -o %t/module2.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/merge.cpp -verify -fsyntax-only
+
+//--- header.h
+namespace NS {
+template <int I>
+class A {
+};
+
+template <template <int I_> class T>
+class B {
+};
+}
+
+//--- module1.h
+namespace NS {
+using C = B<A>;
+}
+struct D : NS::C {
+    using Type = NS::C;
+};
+
+//--- module1.cppm
+// inside NS, using C = B<A>
+module;
+#include "header.h"
+#include "module1.h"
+export module module1;
+export using ::D;
+
+//--- module2.h
+namespace NS {
+using C = B<NS::A>;
+}
+struct D : NS::C {
+    using Type = NS::C;
+};
+
+//--- module2.cppm
+// inside NS, using C = B<NS::A>
+module;
+#include "header.h"
+#include "module2.h"
+export module module2;
+export using ::D;
+
+//--- merge.cpp
+// expected-no-diagnostics
+import module1;
+import module2;
+D d;