[ODRHash] Add support for detecting different method properties.

Now print diagnostics for static, virtual, inline, volatile, and const
differences in methods.  Also use DeclarationName instead of IdentifierInfo
for additional robustness in diagnostic printing.

llvm-svn: 296932

4 files changed, 241 insertions, 10 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
index 4d6ffa1b2a8..d6e040a4427 100644
--- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td
@@ -140,7 +140,13 @@ def err_module_odr_violation_mismatch_decl_diff : Error<
   "%select{non-|}5mutable field %4|"
   "field %4 with %select{no|an}5 initalizer|"
   "field %4 with an initializer|"
-  "method %4}3">;
+  "method %4|"
+  "method %4 is %select{not deleted|deleted}5|"
+  "method %4 is %select{|pure }5%select{not virtual|virtual}6|"
+  "method %4 is %select{not static|static}5|"
+  "method %4 is %select{not volatile|volatile}5|"
+  "method %4 is %select{not const|const}5|"
+  "method %4 is %select{not inline|inline}5}3">;
 
 def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found "
   "%select{"
@@ -154,7 +160,13 @@ def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found "
   "%select{non-|}3mutable field %2|"
   "field %2 with %select{no|an}3 initializer|"
   "field %2 with a different initializer|"
-  "method %2}1">;
+  "method %2|"
+  "method %2 is %select{not deleted|deleted}3|"
+  "method %2 is %select{|pure }3%select{not virtual|virtual}4|"
+  "method %2 is %select{not static|static}3|"
+  "method %2 is %select{not volatile|volatile}3|"
+  "method %2 is %select{not const|const}3|"
+  "method %2 is %select{not inline|inline}3}1">;
 
 def warn_module_uses_date_time : Warning<
   "%select{precompiled header|module}0 uses __DATE__ or __TIME__">,
diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp
index 78a3eeca034..a0b870f4dac 100644
--- a/clang/lib/AST/ODRHash.cpp
+++ b/clang/lib/AST/ODRHash.cpp
@@ -160,7 +160,7 @@ public:
   }
 
   void VisitNamedDecl(const NamedDecl *D) {
-    AddIdentifierInfo(D->getIdentifier());
+    Hash.AddDeclarationName(D->getDeclName());
     Inherited::VisitNamedDecl(D);
   }
 
@@ -196,10 +196,19 @@ public:
   }
 
   void VisitFunctionDecl(const FunctionDecl *D) {
+    ID.AddInteger(D->getStorageClass());
+    Hash.AddBoolean(D->isInlineSpecified());
+    Hash.AddBoolean(D->isVirtualAsWritten());
+    Hash.AddBoolean(D->isPure());
+    Hash.AddBoolean(D->isDeletedAsWritten());
+
     Inherited::VisitFunctionDecl(D);
   }
 
   void VisitCXXMethodDecl(const CXXMethodDecl *D) {
+    Hash.AddBoolean(D->isConst());
+    Hash.AddBoolean(D->isVolatile());
+
     Inherited::VisitCXXMethodDecl(D);
   }
 };
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index fc23e4f7754..c6564d666b2 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9067,6 +9067,12 @@ void ASTReader::diagnoseOdrViolations() {
         FieldSingleInitializer,
         FieldDifferentInitializers,
         MethodName,
+        MethodDeleted,
+        MethodVirtual,
+        MethodStatic,
+        MethodVolatile,
+        MethodConst,
+        MethodInline,
       };
 
       // These lambdas have the common portions of the ODR diagnostics.  This
@@ -9290,16 +9296,103 @@ void ASTReader::diagnoseOdrViolations() {
       case CXXMethod: {
         const CXXMethodDecl *FirstMethod = cast<CXXMethodDecl>(FirstDecl);
         const CXXMethodDecl *SecondMethod = cast<CXXMethodDecl>(SecondDecl);
-        IdentifierInfo *FirstII = FirstMethod->getIdentifier();
-        IdentifierInfo *SecondII = SecondMethod->getIdentifier();
-        if (FirstII->getName() != SecondII->getName()) {
+        auto FirstName = FirstMethod->getDeclName();
+        auto SecondName = SecondMethod->getDeclName();
+        if (FirstName != SecondName) {
           ODRDiagError(FirstMethod->getLocation(),
                        FirstMethod->getSourceRange(), MethodName)
-              << FirstII;
+              << FirstName;
           ODRDiagNote(SecondMethod->getLocation(),
                       SecondMethod->getSourceRange(), MethodName)
-              << SecondII;
+              << SecondName;
+
+          Diagnosed = true;
+          break;
+        }
+
+        const bool FirstDeleted = FirstMethod->isDeleted();
+        const bool SecondDeleted = SecondMethod->isDeleted();
+        if (FirstDeleted != SecondDeleted) {
+          ODRDiagError(FirstMethod->getLocation(),
+                       FirstMethod->getSourceRange(), MethodDeleted)
+              << FirstName << FirstDeleted;
+
+          ODRDiagNote(SecondMethod->getLocation(),
+                      SecondMethod->getSourceRange(), MethodDeleted)
+              << SecondName << SecondDeleted;
+          Diagnosed = true;
+          break;
+        }
+
+        const bool FirstVirtual = FirstMethod->isVirtualAsWritten();
+        const bool SecondVirtual = SecondMethod->isVirtualAsWritten();
+        const bool FirstPure = FirstMethod->isPure();
+        const bool SecondPure = SecondMethod->isPure();
+        if ((FirstVirtual || SecondVirtual) &&
+            (FirstVirtual != SecondVirtual || FirstPure != SecondPure)) {
+          ODRDiagError(FirstMethod->getLocation(),
+                       FirstMethod->getSourceRange(), MethodVirtual)
+              << FirstName << FirstPure << FirstVirtual;
+          ODRDiagNote(SecondMethod->getLocation(),
+                      SecondMethod->getSourceRange(), MethodVirtual)
+              << SecondName << SecondPure << SecondVirtual;
+          Diagnosed = true;
+          break;
+        }
 
+        // CXXMethodDecl::isStatic uses the canonical Decl.  With Decl merging,
+        // FirstDecl is the canonical Decl of SecondDecl, so the storage
+        // class needs to be checked instead.
+        const auto FirstStorage = FirstMethod->getStorageClass();
+        const auto SecondStorage = SecondMethod->getStorageClass();
+        const bool FirstStatic = FirstStorage == SC_Static;
+        const bool SecondStatic = SecondStorage == SC_Static;
+        if (FirstStatic != SecondStatic) {
+          ODRDiagError(FirstMethod->getLocation(),
+                       FirstMethod->getSourceRange(), MethodStatic)
+              << FirstName << FirstStatic;
+          ODRDiagNote(SecondMethod->getLocation(),
+                      SecondMethod->getSourceRange(), MethodStatic)
+              << SecondName << SecondStatic;
+          Diagnosed = true;
+          break;
+        }
+
+        const bool FirstVolatile = FirstMethod->isVolatile();
+        const bool SecondVolatile = SecondMethod->isVolatile();
+        if (FirstVolatile != SecondVolatile) {
+          ODRDiagError(FirstMethod->getLocation(),
+                       FirstMethod->getSourceRange(), MethodVolatile)
+              << FirstName << FirstVolatile;
+          ODRDiagNote(SecondMethod->getLocation(),
+                      SecondMethod->getSourceRange(), MethodVolatile)
+              << SecondName << SecondVolatile;
+          Diagnosed = true;
+          break;
+        }
+
+        const bool FirstConst = FirstMethod->isConst();
+        const bool SecondConst = SecondMethod->isConst();
+        if (FirstConst != SecondConst) {
+          ODRDiagError(FirstMethod->getLocation(),
+                       FirstMethod->getSourceRange(), MethodConst)
+              << FirstName << FirstConst;
+          ODRDiagNote(SecondMethod->getLocation(),
+                      SecondMethod->getSourceRange(), MethodConst)
+              << SecondName << SecondConst;
+          Diagnosed = true;
+          break;
+        }
+
+        const bool FirstInline = FirstMethod->isInlineSpecified();
+        const bool SecondInline = SecondMethod->isInlineSpecified();
+        if (FirstInline != SecondInline) {
+          ODRDiagError(FirstMethod->getLocation(),
+                       FirstMethod->getSourceRange(), MethodInline)
+              << FirstName << FirstInline;
+          ODRDiagNote(SecondMethod->getLocation(),
+                      SecondMethod->getSourceRange(), MethodInline)
+              << SecondName << SecondInline;
           Diagnosed = true;
           break;
         }
diff --git a/clang/test/Modules/odr_hash.cpp b/clang/test/Modules/odr_hash.cpp
index c1ea3b034ff..572d24a077f 100644
--- a/clang/test/Modules/odr_hash.cpp
+++ b/clang/test/Modules/odr_hash.cpp
@@ -308,6 +308,99 @@ S2 s2;
 // expected-error@second.h:* {{'Method::S2' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'B'}}
 // expected-note@first.h:* {{but in 'FirstModule' found method 'A'}}
 #endif
+
+#if defined(FIRST)
+struct S3 {
+  static void A() {}
+};
+#elif defined(SECOND)
+struct S3 {
+  void A() {}
+};
+#else
+S3 s3;
+// expected-error@second.h:* {{'Method::S3' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' is not static}}
+// expected-note@first.h:* {{but in 'FirstModule' found method 'A' is static}}
+#endif
+
+#if defined(FIRST)
+struct S4 {
+  virtual void A() {}
+  void B() {}
+};
+#elif defined(SECOND)
+struct S4 {
+  void A() {}
+  virtual void B() {}
+};
+#else
+S4 s4;
+// expected-error@second.h:* {{'Method::S4' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' is not virtual}}
+// expected-note@first.h:* {{but in 'FirstModule' found method 'A' is virtual}}
+#endif
+
+#if defined(FIRST)
+struct S5 {
+  virtual void A() = 0;
+  virtual void B() {};
+};
+#elif defined(SECOND)
+struct S5 {
+  virtual void A() {}
+  virtual void B() = 0;
+};
+#else
+S5 *s5;
+// expected-error@second.h:* {{'Method::S5' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' is virtual}}
+// expected-note@first.h:* {{but in 'FirstModule' found method 'A' is pure virtual}}
+#endif
+
+#if defined(FIRST)
+struct S6 {
+  inline void A() {}
+};
+#elif defined(SECOND)
+struct S6 {
+  void A() {}
+};
+#else
+S6 s6;
+// expected-error@second.h:* {{'Method::S6' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' is not inline}}
+// expected-note@first.h:* {{but in 'FirstModule' found method 'A' is inline}}
+#endif
+
+#if defined(FIRST)
+struct S7 {
+  void A() volatile {}
+  void A() {}
+};
+#elif defined(SECOND)
+struct S7 {
+  void A() {}
+  void A() volatile {}
+};
+#else
+S7 s7;
+// expected-error@second.h:* {{'Method::S7' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' is not volatile}}
+// expected-note@first.h:* {{but in 'FirstModule' found method 'A' is volatile}}
+#endif
+
+#if defined(FIRST)
+struct S8 {
+  void A() const {}
+  void A() {}
+};
+#elif defined(SECOND)
+struct S8 {
+  void A() {}
+  void A() const {}
+};
+#else
+S8 s8;
+// expected-error@second.h:* {{'Method::S8' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' is not const}}
+// expected-note@first.h:* {{but in 'FirstModule' found method 'A' is const}}
+#endif
+
 }  // namespace Method
 
 // Naive parsing of AST can lead to cycles in processing.  Ensure
@@ -359,6 +452,12 @@ struct S {
   mutable int c = sizeof(x + y);
 
   void method() {}
+  static void static_method() {}
+  virtual void virtual_method() {}
+  virtual void pure_virtual_method() = 0;
+  inline void inline_method() {}
+  void volatile_method() volatile {}
+  void const_method() const {}
 };
 #elif defined(SECOND)
 typedef int INT;
@@ -381,9 +480,15 @@ struct S {
   mutable int c = sizeof(x + y);
 
   void method() {}
+  static void static_method() {}
+  virtual void virtual_method() {}
+  virtual void pure_virtual_method() = 0;
+  inline void inline_method() {}
+  void volatile_method() volatile {}
+  void const_method() const {}
 };
 #else
-S s;
+S *s;
 #endif
 
 #if defined(FIRST)
@@ -407,6 +512,12 @@ struct T {
   mutable int c = sizeof(x + y);
 
   void method() {}
+  static void static_method() {}
+  virtual void virtual_method() {}
+  virtual void pure_virtual_method() = 0;
+  inline void inline_method() {}
+  void volatile_method() volatile {}
+  void const_method() const {}
 
   private:
 };
@@ -431,11 +542,17 @@ struct T {
   mutable int c = sizeof(x + y);
 
   void method() {}
+  static void static_method() {}
+  virtual void virtual_method() {}
+  virtual void pure_virtual_method() = 0;
+  inline void inline_method() {}
+  void volatile_method() volatile {}
+  void const_method() const {}
 
   public:
 };
 #else
-T t;
+T *t;
 // expected-error@second.h:* {{'AllDecls::T' has different definitions in different modules; first difference is definition in module 'SecondModule' found public access specifier}}
 // expected-note@first.h:* {{but in 'FirstModule' found private access specifier}}
 #endif