r311519 - [ODRHash] Diagnose differing template parameters.

Richard Trieu via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 22 19:43:59 PDT 2017


Author: rtrieu
Date: Tue Aug 22 19:43:59 2017
New Revision: 311519

URL: http://llvm.org/viewvc/llvm-project?rev=311519&view=rev
Log:
[ODRHash] Diagnose differing template parameters.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
    cfe/trunk/lib/AST/ODRHash.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/test/Modules/odr_hash.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=311519&r1=311518&r2=311519&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Tue Aug 22 19:43:59 2017
@@ -122,6 +122,24 @@ def note_second_module_difference : Note
 def err_module_odr_violation_different_instantiations : Error<
   "instantiation of %q0 is different in different modules">;
 
+def err_module_odr_violation_template_parameter : Error <
+  "%q0 has different definitions in different modules; first difference is "
+  "%select{definition in module '%2'|defined here}1 found "
+  "%select{"
+  "unnamed template parameter|"
+  "template parameter %4|"
+  "template parameter with %select{no |}4default argument|"
+  "template parameter with default argument}3">;
+
+
+def note_module_odr_violation_template_parameter : Note <
+  "but in '%0' found "
+  "%select{"
+  "unnamed template parameter %2|"
+  "template parameter %2|"
+  "template parameter with %select{no |}2default argument|"
+  "template parameter with different default argument}1">;
+
 def err_module_odr_violation_mismatch_decl : Error<
   "%q0 has different definitions in different modules; first difference is "
   "%select{definition in module '%2'|defined here}1 found "

Modified: cfe/trunk/lib/AST/ODRHash.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=311519&r1=311518&r2=311519&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ODRHash.cpp (original)
+++ cfe/trunk/lib/AST/ODRHash.cpp Tue Aug 22 19:43:59 2017
@@ -158,7 +158,14 @@ void ODRHash::AddTemplateArgument(Templa
   }
 }
 
-void ODRHash::AddTemplateParameterList(const TemplateParameterList *TPL) {}
+void ODRHash::AddTemplateParameterList(const TemplateParameterList *TPL) {
+  assert(TPL && "Expecting non-null pointer.");
+
+  ID.AddInteger(TPL->size());
+  for (auto *ND : TPL->asArray()) {
+    AddSubDecl(ND);
+  }
+}
 
 void ODRHash::clear() {
   DeclMap.clear();
@@ -236,6 +243,10 @@ public:
     }
   }
 
+  void AddTemplateArgument(TemplateArgument TA) {
+    Hash.AddTemplateArgument(TA);
+  }
+
   void Visit(const Decl *D) {
     ID.AddInteger(D->getKind());
     Inherited::Visit(D);
@@ -343,6 +354,42 @@ public:
       AddDecl(D->getFriendDecl());
     }
   }
+
+  void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
+    // Only care about default arguments as part of the definition.
+    const bool hasDefaultArgument =
+        D->hasDefaultArgument() && !D->defaultArgumentWasInherited();
+    Hash.AddBoolean(hasDefaultArgument);
+    if (hasDefaultArgument) {
+      AddTemplateArgument(D->getDefaultArgument());
+    }
+
+    Inherited::VisitTemplateTypeParmDecl(D);
+  }
+
+  void VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D) {
+    // Only care about default arguments as part of the definition.
+    const bool hasDefaultArgument =
+        D->hasDefaultArgument() && !D->defaultArgumentWasInherited();
+    Hash.AddBoolean(hasDefaultArgument);
+    if (hasDefaultArgument) {
+      AddStmt(D->getDefaultArgument());
+    }
+
+    Inherited::VisitNonTypeTemplateParmDecl(D);
+  }
+
+  void VisitTemplateTemplateParmDecl(const TemplateTemplateParmDecl *D) {
+    // Only care about default arguments as part of the definition.
+    const bool hasDefaultArgument =
+        D->hasDefaultArgument() && !D->defaultArgumentWasInherited();
+    Hash.AddBoolean(hasDefaultArgument);
+    if (hasDefaultArgument) {
+      AddTemplateArgument(D->getDefaultArgument().getArgument());
+    }
+
+    Inherited::VisitTemplateTemplateParmDecl(D);
+  }
 };
 } // namespace
 
@@ -403,6 +450,12 @@ void ODRHash::AddCXXRecordDecl(const CXX
   for (auto SubDecl : Decls) {
     AddSubDecl(SubDecl);
   }
+
+  const ClassTemplateDecl *TD = Record->getDescribedClassTemplate();
+  AddBoolean(TD);
+  if (TD) {
+    AddTemplateParameterList(TD->getTemplateParameters());
+  }
 }
 
 void ODRHash::AddDecl(const Decl *D) {

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=311519&r1=311518&r2=311519&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Aug 22 19:43:59 2017
@@ -9274,6 +9274,199 @@ void ASTReader::diagnoseOdrViolations()
 
       std::string SecondModule = getOwningModuleNameForDiagnostic(SecondRecord);
       using DeclHashes = llvm::SmallVector<std::pair<Decl *, unsigned>, 4>;
+
+      const ClassTemplateDecl *FirstTemplate =
+          FirstRecord->getDescribedClassTemplate();
+      const ClassTemplateDecl *SecondTemplate =
+          SecondRecord->getDescribedClassTemplate();
+
+      assert(!FirstTemplate == !SecondTemplate &&
+             "Both pointers should be null or non-null");
+
+      enum ODRTemplateDifference {
+        ParamEmptyName,
+        ParamName,
+        ParamSingleDefaultArgument,
+        ParamDifferentDefaultArgument,
+      };
+
+      if (FirstTemplate && SecondTemplate) {
+        DeclHashes FirstTemplateHashes;
+        DeclHashes SecondTemplateHashes;
+        ODRHash Hash;
+
+        auto PopulateTemplateParameterHashs =
+            [&Hash](DeclHashes &Hashes, const ClassTemplateDecl *TD) {
+              for (auto *D : TD->getTemplateParameters()->asArray()) {
+                Hash.clear();
+                Hash.AddSubDecl(D);
+                Hashes.emplace_back(D, Hash.CalculateHash());
+              }
+            };
+
+        PopulateTemplateParameterHashs(FirstTemplateHashes, FirstTemplate);
+        PopulateTemplateParameterHashs(SecondTemplateHashes, SecondTemplate);
+
+        assert(FirstTemplateHashes.size() == SecondTemplateHashes.size() &&
+               "Number of template parameters should be equal.");
+
+        auto FirstIt = FirstTemplateHashes.begin();
+        auto FirstEnd = FirstTemplateHashes.end();
+        auto SecondIt = SecondTemplateHashes.begin();
+        for (; FirstIt != FirstEnd; ++FirstIt, ++SecondIt) {
+          if (FirstIt->second == SecondIt->second)
+            continue;
+
+          auto ODRDiagError = [FirstRecord, &FirstModule,
+                               this](SourceLocation Loc, SourceRange Range,
+                                     ODRTemplateDifference DiffType) {
+            return Diag(Loc, diag::err_module_odr_violation_template_parameter)
+                   << FirstRecord << FirstModule.empty() << FirstModule << Range
+                   << DiffType;
+          };
+          auto ODRDiagNote = [&SecondModule,
+                              this](SourceLocation Loc, SourceRange Range,
+                                    ODRTemplateDifference DiffType) {
+            return Diag(Loc, diag::note_module_odr_violation_template_parameter)
+                   << SecondModule << Range << DiffType;
+          };
+
+          const NamedDecl* FirstDecl = cast<NamedDecl>(FirstIt->first);
+          const NamedDecl* SecondDecl = cast<NamedDecl>(SecondIt->first);
+
+          assert(FirstDecl->getKind() == SecondDecl->getKind() &&
+                 "Parameter Decl's should be the same kind.");
+
+          DeclarationName FirstName = FirstDecl->getDeclName();
+          DeclarationName SecondName = SecondDecl->getDeclName();
+
+          if (FirstName != SecondName) {
+            const bool FirstNameEmpty =
+                FirstName.isIdentifier() && !FirstName.getAsIdentifierInfo();
+            const bool SecondNameEmpty =
+                SecondName.isIdentifier() && !SecondName.getAsIdentifierInfo();
+            assert((!FirstNameEmpty || !SecondNameEmpty) &&
+                   "Both template parameters cannot be unnamed.");
+            ODRDiagError(FirstDecl->getLocation(), FirstDecl->getSourceRange(),
+                         FirstNameEmpty ? ParamEmptyName : ParamName)
+                << FirstName;
+            ODRDiagNote(SecondDecl->getLocation(), SecondDecl->getSourceRange(),
+                        SecondNameEmpty ? ParamEmptyName : ParamName)
+                << SecondName;
+            break;
+          }
+
+          switch (FirstDecl->getKind()) {
+          default:
+            llvm_unreachable("Invalid template parameter type.");
+          case Decl::TemplateTypeParm: {
+            const auto *FirstParam = cast<TemplateTypeParmDecl>(FirstDecl);
+            const auto *SecondParam = cast<TemplateTypeParmDecl>(SecondDecl);
+            const bool HasFirstDefaultArgument =
+                FirstParam->hasDefaultArgument() &&
+                !FirstParam->defaultArgumentWasInherited();
+            const bool HasSecondDefaultArgument =
+                SecondParam->hasDefaultArgument() &&
+                !SecondParam->defaultArgumentWasInherited();
+
+            if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
+              ODRDiagError(FirstDecl->getLocation(),
+                           FirstDecl->getSourceRange(),
+                           ParamSingleDefaultArgument)
+                  << HasFirstDefaultArgument;
+              ODRDiagNote(SecondDecl->getLocation(),
+                          SecondDecl->getSourceRange(),
+                          ParamSingleDefaultArgument)
+                  << HasSecondDefaultArgument;
+              break;
+            }
+
+            assert(HasFirstDefaultArgument && HasSecondDefaultArgument &&
+                   "Expecting default arguments.");
+
+            ODRDiagError(FirstDecl->getLocation(), FirstDecl->getSourceRange(),
+                         ParamDifferentDefaultArgument);
+            ODRDiagNote(SecondDecl->getLocation(), SecondDecl->getSourceRange(),
+                        ParamDifferentDefaultArgument);
+
+            break;
+          }
+          case Decl::NonTypeTemplateParm: {
+            const auto *FirstParam = cast<NonTypeTemplateParmDecl>(FirstDecl);
+            const auto *SecondParam = cast<NonTypeTemplateParmDecl>(SecondDecl);
+            const bool HasFirstDefaultArgument =
+                FirstParam->hasDefaultArgument() &&
+                !FirstParam->defaultArgumentWasInherited();
+            const bool HasSecondDefaultArgument =
+                SecondParam->hasDefaultArgument() &&
+                !SecondParam->defaultArgumentWasInherited();
+
+            if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
+              ODRDiagError(FirstDecl->getLocation(),
+                           FirstDecl->getSourceRange(),
+                           ParamSingleDefaultArgument)
+                  << HasFirstDefaultArgument;
+              ODRDiagNote(SecondDecl->getLocation(),
+                          SecondDecl->getSourceRange(),
+                          ParamSingleDefaultArgument)
+                  << HasSecondDefaultArgument;
+              break;
+            }
+
+            assert(HasFirstDefaultArgument && HasSecondDefaultArgument &&
+                   "Expecting default arguments.");
+
+            ODRDiagError(FirstDecl->getLocation(), FirstDecl->getSourceRange(),
+                         ParamDifferentDefaultArgument);
+            ODRDiagNote(SecondDecl->getLocation(), SecondDecl->getSourceRange(),
+                        ParamDifferentDefaultArgument);
+
+            break;
+          }
+          case Decl::TemplateTemplateParm: {
+            const auto *FirstParam = cast<TemplateTemplateParmDecl>(FirstDecl);
+            const auto *SecondParam =
+                cast<TemplateTemplateParmDecl>(SecondDecl);
+            const bool HasFirstDefaultArgument =
+                FirstParam->hasDefaultArgument() &&
+                !FirstParam->defaultArgumentWasInherited();
+            const bool HasSecondDefaultArgument =
+                SecondParam->hasDefaultArgument() &&
+                !SecondParam->defaultArgumentWasInherited();
+
+            if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
+              ODRDiagError(FirstDecl->getLocation(),
+                           FirstDecl->getSourceRange(),
+                           ParamSingleDefaultArgument)
+                  << HasFirstDefaultArgument;
+              ODRDiagNote(SecondDecl->getLocation(),
+                          SecondDecl->getSourceRange(),
+                          ParamSingleDefaultArgument)
+                  << HasSecondDefaultArgument;
+              break;
+            }
+
+            assert(HasFirstDefaultArgument && HasSecondDefaultArgument &&
+                   "Expecting default arguments.");
+
+            ODRDiagError(FirstDecl->getLocation(), FirstDecl->getSourceRange(),
+                         ParamDifferentDefaultArgument);
+            ODRDiagNote(SecondDecl->getLocation(), SecondDecl->getSourceRange(),
+                        ParamDifferentDefaultArgument);
+
+            break;
+          }
+          }
+
+          break;
+        }
+
+        if (FirstIt != FirstEnd) {
+          Diagnosed = true;
+          break;
+        }
+      }
+
       DeclHashes FirstHashes;
       DeclHashes SecondHashes;
       ODRHash Hash;

Modified: cfe/trunk/test/Modules/odr_hash.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=311519&r1=311518&r2=311519&view=diff
==============================================================================
--- cfe/trunk/test/Modules/odr_hash.cpp (original)
+++ cfe/trunk/test/Modules/odr_hash.cpp Tue Aug 22 19:43:59 2017
@@ -1500,6 +1500,83 @@ S5 s5;
 // expected-note at first.h:* {{but in 'FirstModule' found friend function 'T5a'}}
 #endif
 }
+
+namespace TemplateParameters {
+#if defined(FIRST)
+template <class A>
+struct S1 {};
+#elif defined(SECOND)
+template <class B>
+struct S1 {};
+#else
+using TemplateParameters::S1;
+// expected-error at second.h:* {{'TemplateParameters::S1' has different definitions in different modules; first difference is definition in module 'SecondModule' found template parameter 'B'}}
+// expected-note at first.h:* {{but in 'FirstModule' found template parameter 'A'}}
+#endif
+
+#if defined(FIRST)
+template <class A = double>
+struct S2 {};
+#elif defined(SECOND)
+template <class A = int>
+struct S2 {};
+#else
+using TemplateParameters::S2;
+// expected-error at second.h:* {{'TemplateParameters::S2' has different definitions in different modules; first difference is definition in module 'SecondModule' found template parameter with default argument}}
+// expected-note at first.h:* {{but in 'FirstModule' found template parameter with different default argument}}
+#endif
+
+#if defined(FIRST)
+template <class A = int>
+struct S3 {};
+#elif defined(SECOND)
+template <class A>
+struct S3 {};
+#else
+using TemplateParameters::S3;
+// expected-error at second.h:* {{'TemplateParameters::S3' has different definitions in different modules; first difference is definition in module 'SecondModule' found template parameter with no default argument}}
+// expected-note at first.h:* {{but in 'FirstModule' found template parameter with default argument}}
+#endif
+
+#if defined(FIRST)
+template <int A>
+struct S4 {};
+#elif defined(SECOND)
+template <int A = 2>
+struct S4 {};
+#else
+using TemplateParameters::S4;
+// expected-error at second.h:* {{'TemplateParameters::S4' has different definitions in different modules; first difference is definition in module 'SecondModule' found template parameter with default argument}}
+// expected-note at first.h:* {{but in 'FirstModule' found template parameter with no default argument}}
+#endif
+
+#if defined(FIRST)
+template <int> class S5_first {};
+template <template<int> class A = S5_first>
+struct S5 {};
+#elif defined(SECOND)
+template <int> class S5_second {};
+template <template<int> class A = S5_second>
+struct S5 {};
+#else
+using TemplateParameters::S5;
+// expected-error at second.h:* {{'TemplateParameters::S5' has different definitions in different modules; first difference is definition in module 'SecondModule' found template parameter with default argument}}
+// expected-note at first.h:* {{but in 'FirstModule' found template parameter with different default argument}}
+#endif
+
+#if defined(FIRST)
+template <class A>
+struct S6 {};
+#elif defined(SECOND)
+template <class>
+struct S6 {};
+#else
+using TemplateParameters::S6;
+// expected-error at second.h:* {{'TemplateParameters::S6' has different definitions in different modules; first difference is definition in module 'SecondModule' found unnamed template parameter}}
+// expected-note at first.h:* {{but in 'FirstModule' found template parameter 'A'}}
+#endif
+}  // namespace TemplateParameters
+
 // Interesting cases that should not cause errors.  struct S should not error
 // while struct T should error at the access specifier mismatch at the end.
 namespace AllDecls {




More information about the cfe-commits mailing list