[ODRHash] Add handling of TypedefType and DeclarationName

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

llvm-svn: 296078

clang/include/clang/Basic/DiagnosticSerializationKinds.td

@@ -133,14 +133,14 @@ def err_module_odr_violation_mismatch_decl_diff : Error<
   "static assert with condition|"
   "static assert with message|"
   "static assert with %select{|no }4message|"
-  "field %4}3">;
+  "field %4|field %4 with type %5}3">;
 
 def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found "
   "%select{"
   "static assert with different condition|"
   "static assert with different message|"
   "static assert with %select{|no }2message|"
-  "field %2}1">;
+  "field %2|field %2 with type %3}1">;
 
 def warn_module_uses_date_time : Warning<
   "%select{precompiled header|module}0 uses __DATE__ or __TIME__">,

clang/lib/AST/ODRHash.cpp

@@ -32,9 +32,57 @@ void ODRHash::AddIdentifierInfo(const IdentifierInfo *II) {
   ID.AddString(II->getName());
 }
 
+void ODRHash::AddDeclarationName(DeclarationName Name) {
+  AddBoolean(Name.isEmpty());
+  if (Name.isEmpty())
+    return;
+
+  auto Kind = Name.getNameKind();
+  ID.AddInteger(Kind);
+  switch (Kind) {
+  case DeclarationName::Identifier:
+    AddIdentifierInfo(Name.getAsIdentifierInfo());
+    break;
+  case DeclarationName::ObjCZeroArgSelector:
+  case DeclarationName::ObjCOneArgSelector:
+  case DeclarationName::ObjCMultiArgSelector: {
+    Selector S = Name.getObjCSelector();
+    AddBoolean(S.isNull());
+    AddBoolean(S.isKeywordSelector());
+    AddBoolean(S.isUnarySelector());
+    unsigned NumArgs = S.getNumArgs();
+    for (unsigned i = 0; i < NumArgs; ++i) {
+      AddIdentifierInfo(S.getIdentifierInfoForSlot(i));
+    }
+    break;
+  }
+  case DeclarationName::CXXConstructorName:
+  case DeclarationName::CXXDestructorName:
+    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: {
+    auto *Template = Name.getCXXDeductionGuideTemplate();
+    AddBoolean(Template);
+    if (Template) {
+      AddDecl(Template);
+    }
+  }
+  }
+}
+
 void ODRHash::AddNestedNameSpecifier(const NestedNameSpecifier *NNS) {}
 void ODRHash::AddTemplateName(TemplateName Name) {}
-void ODRHash::AddDeclarationName(DeclarationName Name) {}
 void ODRHash::AddTemplateArgument(TemplateArgument TA) {}
 void ODRHash::AddTemplateParameterList(const TemplateParameterList *TPL) {}
 
@@ -192,6 +240,10 @@ void ODRHash::AddDecl(const Decl *D) {
   }
 
   ID.AddInteger(D->getKind());
+
+  if (const NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
+    AddDeclarationName(ND->getDeclName());
+  }
 }
 
 // Process a Type pointer.  Add* methods call back into ODRHash while Visit*
@@ -212,6 +264,13 @@ public:
     }
   }
 
+  void AddDecl(Decl *D) {
+    Hash.AddBoolean(D);
+    if (D) {
+      Hash.AddDecl(D);
+    }
+  }
+
   void Visit(const Type *T) {
     ID.AddInteger(T->getTypeClass());
     Inherited::Visit(T);
@@ -223,6 +282,11 @@ public:
     ID.AddInteger(T->getKind());
     VisitType(T);
   }
+
+  void VisitTypedefType(const TypedefType *T) {
+    AddDecl(T->getDecl());
+    VisitType(T);
+  }
 };
 
 void ODRHash::AddType(const Type *T) {

clang/lib/Serialization/ASTReader.cpp

@@ -9062,6 +9062,7 @@ void ASTReader::diagnoseOdrViolations() {
         StaticAssertMessage,
         StaticAssertOnlyMessage,
         FieldName,
+        FieldTypeName,
       };
 
       // These lambdas have the common portions of the ODR diagnostics.  This
@@ -9086,6 +9087,12 @@ void ASTReader::diagnoseOdrViolations() {
         return Hash.CalculateHash();
       };
 
+      auto ComputeDeclNameODRHash = [&Hash](const DeclarationName Name) {
+        Hash.clear();
+        Hash.AddDeclarationName(Name);
+        return Hash.CalculateHash();
+      };
+
       switch (FirstDiffType) {
       case Other:
       case EndOfClass:
@@ -9166,6 +9173,46 @@ void ASTReader::diagnoseOdrViolations() {
           Diagnosed = true;
           break;
         }
+
+        assert(
+            Context.hasSameType(FirstField->getType(), SecondField->getType()));
+
+        QualType FirstType = FirstField->getType();
+        QualType SecondType = SecondField->getType();
+        const TypedefType *FirstTypedef = dyn_cast<TypedefType>(FirstType);
+        const TypedefType *SecondTypedef = dyn_cast<TypedefType>(SecondType);
+
+        if ((FirstTypedef && !SecondTypedef) ||
+            (!FirstTypedef && SecondTypedef)) {
+          ODRDiagError(FirstField->getLocation(), FirstField->getSourceRange(),
+                       FieldTypeName)
+              << FirstII << FirstType;
+          ODRDiagNote(SecondField->getLocation(), SecondField->getSourceRange(),
+                      FieldTypeName)
+              << SecondII << SecondType;
+
+          Diagnosed = true;
+          break;
+        }
+
+        if (FirstTypedef && SecondTypedef) {
+          unsigned FirstHash = ComputeDeclNameODRHash(
+              FirstTypedef->getDecl()->getDeclName());
+          unsigned SecondHash = ComputeDeclNameODRHash(
+              SecondTypedef->getDecl()->getDeclName());
+          if (FirstHash != SecondHash) {
+            ODRDiagError(FirstField->getLocation(),
+                         FirstField->getSourceRange(), FieldTypeName)
+                << FirstII << FirstType;
+            ODRDiagNote(SecondField->getLocation(),
+                        SecondField->getSourceRange(), FieldTypeName)
+                << SecondII << SecondType;
+
+            Diagnosed = true;
+            break;
+          }
+        }
+
         break;
       }
       }

clang/test/Modules/odr_hash.cpp

@@ -162,6 +162,36 @@ S3 s3;
 // expected-error@first.h:* {{'Field::S3::x' from module 'FirstModule' is not present in definition of 'Field::S3' in module 'SecondModule'}}
 // expected-note@second.h:* {{declaration of 'x' does not match}}
 #endif
+
+#if defined(FIRST)
+typedef int A;
+struct S4 {
+  A x;
+};
+
+struct S5 {
+  A x;
+};
+#elif defined(SECOND)
+typedef int B;
+struct S4 {
+  B x;
+};
+
+struct S5 {
+  int x;
+};
+#else
+S4 s4;
+// expected-error@second.h:* {{'Field::S4' has different definitions in different modules; first difference is definition in module 'SecondModule' found field 'x' with type 'B' (aka 'int')}}
+// expected-note@first.h:* {{but in 'FirstModule' found field 'x' with type 'A' (aka 'int')}}
+
+S5 s5;
+// expected-error@second.h:* {{'Field::S5' has different definitions in different modules; first difference is definition in module 'SecondModule' found field 'x' with type 'int'}}
+// expected-note@first.h:* {{but in 'FirstModule' found field 'x' with type 'A' (aka 'int')}}
+#endif
+
+
 }  // namespace Field
 
 // Naive parsing of AST can lead to cycles in processing.  Ensure
@@ -193,6 +223,7 @@ struct NestedNamespaceSpecifier {};
 // while struct T should error at the access specifier mismatch at the end.
 namespace AllDecls {
 #if defined(FIRST)
+typedef int INT;
 struct S {
   public:
   private:
@@ -203,8 +234,11 @@ struct S {
 
   int x;
   double y;
+
+  INT z;
 };
 #elif defined(SECOND)
+typedef int INT;
 struct S {
   public:
   private:
@@ -215,12 +249,15 @@ struct S {
 
   int x;
   double y;
+
+  INT z;
 };
 #else
 S s;
 #endif
 
 #if defined(FIRST)
+typedef int INT;
 struct T {
   public:
   private:
@@ -232,9 +269,12 @@ struct T {
   int x;
   double y;
 
+  INT z;
+
   private:
 };
 #elif defined(SECOND)
+typedef int INT;
 struct T {
   public:
   private:
@@ -246,6 +286,8 @@ struct T {
   int x;
   double y;
 
+  INT z;
+
   public:
 };
 #else