[clang] ODR hashes depth+index and not name of TemplateTypeParm

Change the ODR hashing logic to use the depth+index indices instead
of template parameter names. This prevents spurious ODR errors in
header module builds when the type canonicalization picks up
different expressions - with different template parameter names
(profiling used for canonicalization ignores template parameter
names). This fixes #143152.

This commit removes test assertions from the regression test of
#108866 - the compiler
errors asserted by that test stop firing with this commit. It's
questionable whether these diagnostics were correct. We add a
FIXME to find a better regression test for that fix.

Author: emaxx-google

clang/lib/AST/ODRHash.cpp

@@ -828,7 +828,23 @@ void ODRHash::AddDecl(const Decl *D) {
     return;
   }
 
-  AddDeclarationName(ND->getDeclName());
+  // For template parameters, use depth+index instead of name, because type
+  // canonicalization can change the name of the template parameter.
+  if (auto *TTPD = dyn_cast<TemplateTypeParmDecl>(ND)) {
+    ID.AddInteger(TTPD->getDepth());
+    ID.AddInteger(TTPD->getIndex());
+    AddBoolean(TTPD->isParameterPack());
+  } else if (auto *NTTPD = dyn_cast<NonTypeTemplateParmDecl>(ND)) {
+    ID.AddInteger(NTTPD->getDepth());
+    ID.AddInteger(NTTPD->getIndex());
+    AddBoolean(NTTPD->isParameterPack());
+  } else if (auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(ND)) {
+    ID.AddInteger(TTPD->getDepth());
+    ID.AddInteger(TTPD->getIndex());
+    AddBoolean(TTPD->isParameterPack());
+  } else {
+    AddDeclarationName(ND->getDeclName());
+  }
 
   // If this was a specialization we should take into account its template
   // arguments. This helps to reduce collisions coming when visiting template

clang/test/Modules/odr_hash.cpp

@@ -5164,6 +5164,29 @@ namespace A {
 A::X x;
 #endif
 
+namespace TemplateDecltypeOperator {
+
+#if defined(FIRST) || defined(SECOND)
+template <class T6>
+T6 func();
+#endif
+
+#if defined(SECOND)
+template <class UnrelatedT>
+using UnrelatedAlias = decltype(func<UnrelatedT>())();
+#endif
+
+#if defined(FIRST) || defined(SECOND)
+class A {
+  template <class T6>
+  operator decltype(func<T6>()) () {}
+};
+#else
+A a;
+#endif
+
+}
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST

clang/test/PCH/race-condition.cpp

@@ -31,11 +31,12 @@ constexpr enable_if_t<meta<F>::value == 2, void> midpoint(F) {}
 
 #else
 
-// expected-error@27{{'N::midpoint' has different definitions in different modules; defined here first difference is 1st parameter with type 'F'}}
-// expected-error@24{{'N::midpoint' has different definitions in different modules; defined here first difference is 1st parameter with type 'U'}}
-// expected-note@21{{but in '' found 1st parameter with type 'T'}}
+// FIXME: Change the test to trigger a suitable error: previously the test
+// asserted the ODR error ("'N::midpoint' has different definitions in different
+// modules"), which isn't fully correct as there's only one module, and since a
+// change in the ODR hash calculation this error isn't triggered anymore.
+
 int x = N::something;
-// expected-error@37{{no member named 'something' in namespace 'N'}}
-// expected-note@21{{but in '' found 1st parameter with type 'T'}}
+// expected-error@38{{no member named 'something' in namespace 'N'}}
 
 #endif