[clang][ASTImporter] Import TemplateName correctly

[balazske]

Summary

This is a fix for a problem when multiple template
specializations are created by ASTImporter for the
same specialization. The problem happens if a
TemplateName is imported that points to a template
delcaration (for a template template argument)
(specialization) that has multiple instances in the
declaration chain. If two TemplateName objects contain
different pointers to a template specialization,
these TemplateName objects will have different checksum
even if they point into the same declaration chain.
The problem is fixed if the canonical declaration is used.

[vabridgers]

This patch needs a unit test (as @balazske mentioned). So far, the case we have is too large to be a suitable unittest or lit case - so requires reduction. @balazske , will you be adding this as a unittest or lit case?

Also, I think this patch needs to be integrated with D144273 IMHO, since they address similar issues.

I verified the patch stack D144273, D140562, and D144622 (this one) addresses the problems we've seen as a result of D133468. When the patches are ready, a brief history should be included in the commit header.

[balazske]

I think these patches are fix for separate problems and can be applied independently. It is not better if these are moved into one change. The other patch D144273 is not finished (it can get bigger), and a test for this change is needed. I try still to discover what the exact problem is here, then I can add an unit test.

[balazske]

Added a test case.

[donat.nagy]

Basically LGTM (assuming that the TC passes), I added two minor suggestions, but I'm not opposed to merging this in its current state.

clang/unittests/AST/ASTImporterTest.cpp

8142–8150 ↗

(On Diff #501161)

Nitpick: consider saving this string in a constant instead of repeating it twice.

8201–8202 ↗

(On Diff #501161)

For clarity, perhaps also assert that Spec1 != Spec2.

[balazske]

Updated the test.

[vabridgers]

LGTM

[donat.nagy]

LGTM as well.

clang/lib/AST/ASTImporter.cpp

 //===- ASTImporter.cpp - Importing ASTs from other Contexts ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
ASTImporter::Import(NestedNameSpecifierLoc FromNNS) {
 
   return Builder.getWithLocInContext(getToContext());
 }
 
 Expected<TemplateName> ASTImporter::Import(TemplateName From) {
   switch (From.getKind()) {
   case TemplateName::Template:
     if (ExpectedDecl ToTemplateOrErr = Import(From.getAsTemplateDecl()))
-      return TemplateName(cast<TemplateDecl>(*ToTemplateOrErr));
+      return TemplateName(cast<TemplateDecl>((*ToTemplateOrErr)->getCanonicalDecl()));
     else
       return ToTemplateOrErr.takeError();
 
   case TemplateName::OverloadedTemplate: {
     OverloadedTemplateStorage *FromStorage = From.getAsOverloadedTemplate();
     UnresolvedSet<2> ToTemplates;
     for (auto *I : *FromStorage) {
       if (auto ToOrErr = Import(I))