[ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

[martong]

Summary

A new TypeLocImporter is created to import the TypeLoc values from
TypeSourceInfo. The previous "trivial TypeSourceInfo" is replaced.

[balazske]

clang/lib/AST/ASTImporter.cpp

8109

I think this should have a less-generic name for example TypeLocImporter_IMPORT.

[martong]

IMPORT -> IMPORT_TYPE_LOC

[a_sidorin]

Finally, this FIXME is addressed. Thanks! Are you going to add any tests in the following patches?
It looks almost good to me, but I have a question inline.

clang/lib/AST/ASTImporter.cpp

8109

IMPORT_TYPE_LOC_ELEMENT_OR_RETURN_ERROR.

8238

Does this import interacts well with type loc import partially done at L3258 (VisitFunctionDecl)? Should we merge them?

[martong]

Are you going to add any tests in the following patches?

I don't have any tests for now, but in a later patch we can create them.

clang/lib/AST/ASTImporter.cpp

8238

I think we should check here whether the given ParmVarDecl had been imported previously and if yes then set that with setParam, otherwise it could be set to a nullptr. @balazske what do you think?

[balazske]

clang/lib/AST/ASTImporter.cpp

8238

Probably we could get the FunctionTypeLoc from TInfo (this is the first TypeLoc there?) and set the parameters (that are set to nullptr at line 8055), at this loop in VisitFunctionDecl:

// Set the parameters.
for (auto *Param : Parameters) {
  Param->setOwningFunction(ToFunction);
  ToFunction->addDeclInternal(Param);
}
ToFunction->setParams(Parameters);

[martong]

clang/lib/AST/ASTImporter.cpp

8238

Guys, I moved the TypeLoc import part of VisitFunctionDecl to here. But, this is possible only if first we import the parameters and only then we import the TypeLoc (TypeSourceInfo) in VisitFunctionDecl, so I made that reorganization too.

[martong]

• Move the setting of parameters of FunctionProtoTypeLoc to TypeLocImporter
• Add tests for TypeLoc import in case of functions

[a_sidorin]

The patch looks good. Thanks! I would prefer to see more tests for other TypeLoc cases, but I think they could be added in the following patches.
@shafik Shafik, do you have any suggestions for this patch?

[martong]

@teemperor , @shafik
Guys, could you please take a look on this patch? I don't think that type locations could have any effects on LLDB, but one may never know, so, it is better to have your review too.
Thanks!

[martong]

@shafik
Ping :)

[shafik]

A few comments and I would like @teemperor to give this a look as well but it looks good to me.

clang/lib/AST/ASTImporter.cpp

3281

I am curious, why move this chunk of code up?

clang/unittests/AST/ASTImporterTest.cpp

5850

Maybe I am missing it but these tests don't seem like they cover all the visit methods of TypeLocImporter.

[teemperor]

My only problem is the reimplementation of the default visit methods with (from what I can tell) the same behavior as the default. If you delete all of these then that would also fix Shafik's point that all the custom dispatch code is technically untested in the unit test (I think not testing the dispatch code from the TypeLocVisitor is fine).

clang/lib/AST/ASTImporter.cpp

8133

I know the ASTImporter is doing these reimplementation of the default visitor behavior quite often, but I feel in this case it would be much more readable to omit all these default implementations and just let the TypeLocVisitor do the dispatch to parent class logic. Especially since from what I understand it's unlikely that we need custom logic for all these TypeLoc subclasses in the future? It would also make this code less fragile in case the inheritance hierarchy every gets another intermediate class that might require special handling.

[balazske]

clang/lib/AST/ASTImporter.cpp

3281

See comments at line 8238?

clang/unittests/AST/ASTImporterTest.cpp

5850

It could be useful to add some kind of generic imported AST similarity tests. These can test for things that should match in the imported and original AST, for example source locations.
(The AST does not always match after import but often.) Such a test can catch many import errors with objects that have no specific test.
The test in https://reviews.llvm.org/D60463 is somewhat similar but it uses text dump of the AST, this can be improved. Actually that test was used to discover the import problems of SourceLocation and TypeSourceInfo.

[balazske]

clang/lib/AST/ASTImporter.cpp

8133

It is really better to remove these redundant visit branches.

[martong]

Thanks guys for the review and the comments! :) I am going to address all of them, but only during next week, I am on vacation for the rest of this week.

[martong]

• Remove default visit functions

clang/lib/AST/ASTImporter.cpp

3281

I moved the TypeLoc import part of VisitFunctionDecl to VisitFunctionTypeLoc. But, that was possible only if first we import the parameters and only then we import the TypeLoc (TypeSourceInfo) in VisitFunctionDecl, so I made that reorganization here.

8133

OK, indeed we don't need them. So, I removed these default implementations.

clang/unittests/AST/ASTImporterTest.cpp

5850

I added tests only for the non-trivial cases like FunctionTypeLoc, in that case we moved some code out from VisitFunctionDecl to the typeloc importer.

All the other branches are very trivial and type loc import happens only in the type loc importer. So, I think it would be the best if we could test them in a generic way as @balazske suggests: we could extend the existing unittests to check for typelocations too. But that is not that trivial to do (may require another typeloc visitation in the test, I don't know yet). I'd rather do that in a separate patch, if that's okay.

[shafik]

LGTM

[balazske]

clang/lib/AST/ASTImporter.cpp

8132

This assert is new related to the old code. Are really all TypeLoc's are implemented (probably analyze with CTU a large project)?

[martong]

clang/lib/AST/ASTImporter.cpp

8132

Yeah, I am not going to commit this, until our full CI job is green on the change. That job analyzes Tmux, Redis, Curl, Xercex, Bitcoin and Protobuf and considered as a quite good coverage for the Analyzer.

[martong]

• Handle AttributedTypeLoc

[martong]

• Remove redundant VisitQualifiedTypeLoc and VisitAttributedTypeLoc

[martong]

• Fix the base commit of this patch

[martong]

In D71018#1924604, @martong wrote:

• Remove redundant VisitQualifiedTypeLoc and VisitAttributedTypeLoc

I realized that the mentioned functions are redundant, because we have a while loop which iterates over all related typelocs (getNextTypeLoc).
Also, it is better to remove the llvm_unreachable because there are many TypeLocs that we just don't handle, e.g. TypeOfExprTypeLoc.

clang/lib/AST/ASTImporter.cpp

 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/AST/TypeLocVisitor.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
public:
 
     Expected<FunctionDecl *> FindFunctionTemplateSpecialization(
         FunctionDecl *FromFD);
 
     // Returns true if the given function has a placeholder return type and
     // that type is declared inside the body of the function.
     // E.g. auto f() { struct X{}; return X(); }
     bool hasAutoReturnTypeDeclaredInside(FunctionDecl *D);
+
+    friend class TypeLocImporter;
   };
 
 template <typename InContainerTy>
 Error ASTNodeImporter::ImportTemplateArgumentListInfo(
     SourceLocation FromLAngleLoc, SourceLocation FromRAngleLoc,
     const InContainerTy &Container, TemplateArgumentListInfo &Result) {
   auto ToLAngleLocOrErr = import(FromLAngleLoc);
   if (!ToLAngleLocOrErr)
if (FromEPI.ExceptionSpec.SourceDecl ||
       FunctionProtoType::ExtProtoInfo DefaultEPI;
       FromEPI = DefaultEPI;
       UsedDifferentProtoType = true;
     }
     FromTy = Importer.getFromContext().getFunctionType(
         FromReturnTy, FromFPT->getParamTypes(), FromEPI);
   }
 
+  // Import the function parameters.
+  SmallVector<ParmVarDecl *, 8> Parameters;
+  for (auto P : D->parameters()) {
+    if (Expected<ParmVarDecl *> ToPOrErr = import(P))
+      Parameters.push_back(*ToPOrErr);
+    else
+      return ToPOrErr.takeError();
+  }
+
   QualType T;
   TypeSourceInfo *TInfo;
   SourceLocation ToInnerLocStart, ToEndLoc;
   NestedNameSpecifierLoc ToQualifierLoc;
   Expr *TrailingRequiresClause;
   if (auto Imp = importSeq(
       FromTy, D->getTypeSourceInfo(), D->getInnerLocStart(),
       D->getQualifierLoc(), D->getEndLoc(), D->getTrailingRequiresClause()))
     std::tie(T, TInfo, ToInnerLocStart, ToQualifierLoc, ToEndLoc,
              TrailingRequiresClause) = *Imp;
   else
     return Imp.takeError();
 
-  // Import the function parameters.
-  SmallVector<ParmVarDecl *, 8> Parameters;
-  for (auto P : D->parameters()) {
-    if (Expected<ParmVarDecl *> ToPOrErr = import(P))
-      Parameters.push_back(*ToPOrErr);
-    else
-      return ToPOrErr.takeError();
-  }
-
   // Create the imported function.
   FunctionDecl *ToFunction = nullptr;
   if (auto *FromConstructor = dyn_cast<CXXConstructorDecl>(D)) {
     Expr *ExplicitExpr = nullptr;
     if (FromConstructor->getExplicitSpecifier().getExpr()) {
       auto Imp = importSeq(FromConstructor->getExplicitSpecifier().getExpr());
       if (!Imp)
         return Imp.takeError();
ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   // Set the parameters.
   for (auto *Param : Parameters) {
     Param->setOwningFunction(ToFunction);
     ToFunction->addDeclInternal(Param);
   }
   ToFunction->setParams(Parameters);
 
-  // We need to complete creation of FunctionProtoTypeLoc manually with setting
-  // params it refers to.
-  if (TInfo) {
-    if (auto ProtoLoc =
-        TInfo->getTypeLoc().IgnoreParens().getAs<FunctionProtoTypeLoc>()) {
-      for (unsigned I = 0, N = Parameters.size(); I != N; ++I)
-        ProtoLoc.setParam(I, Parameters[I]);
-    }
-  }
-
   // Import the describing template function, if any.
   if (FromFT) {
     auto ToFTOrErr = import(FromFT);
     if (!ToFTOrErr)
       return ToFTOrErr.takeError();
   }
 
   // Import Ctor initializers.
if (!ToTOrErr)
     return ToTOrErr.takeError();
 
   // Record the imported type.
   ImportedTypes[FromTy] = (*ToTOrErr).getTypePtr();
 
   return ToContext.getQualifiedType(*ToTOrErr, FromT.getLocalQualifiers());
 }
 
+namespace clang {
+
+#define IMPORT_TYPE_LOC_OR_RETURN_ERROR(NAME)                                  \
+  if (auto ImpOrErr = Importer.Import(From.get##NAME()))                       \
+    To.set##NAME(*ImpOrErr);                                                   \
+  else                                                                         \
+    return ImpOrErr.takeError();
+
+// Import TypeLoc information.
+// TypeLocReader in ASTReader.cpp gives hints about what to import.
+// (At some ObjC types not every existing location is copied.)
+class TypeLocImporter : public TypeLocVisitor<TypeLocImporter, Error> {
+  ASTImporter &Importer;
+  TypeLoc ToL;
+
+public:
+  TypeLocImporter(ASTImporter &Importer, TypeLoc ToL)
+      : Importer(Importer), ToL(ToL) {}
+
+  Error VisitTypeLoc(TypeLoc From) {
+    // The visitor does not call directly VisitTypeSpecTypeLoc, because that is
+    // not a leaf node in the hierarchy (i.e. there is no such enum value in
+    // TypeNodes.inc).
+    if (auto FTS = From.getAs<TypeSpecTypeLoc>())
+      return VisitTypeSpecTypeLoc(FTS);
+    return Error::success();
+  }
+
+  Error VisitTypeSpecTypeLoc(TypeSpecTypeLoc From) {
+    auto To = ToL.castAs<TypeSpecTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(NameLoc);
+    return Error::success();
+  }
+
+  Error VisitBuiltinTypeLoc(BuiltinTypeLoc From) {
+    auto To = ToL.castAs<BuiltinTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(BuiltinLoc);
+    return Error::success();
+  }
+
+  Error VisitParenTypeLoc(ParenTypeLoc From) {
+    auto To = ToL.castAs<ParenTypeLoc>();
+
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LParenLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(RParenLoc);
+
+    return Error::success();
+  }
+
+  Error VisitPointerTypeLoc(PointerTypeLoc From) {
+    auto To = ToL.castAs<PointerTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
+    return Error::success();
+  }
+
+  Error VisitBlockPointerTypeLoc(BlockPointerTypeLoc From) {
+    auto To = ToL.castAs<BlockPointerTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(CaretLoc);
+    return Error::success();
+  }
+
+  Error VisitMemberPointerTypeLoc(MemberPointerTypeLoc From) {
+    auto To = ToL.castAs<MemberPointerTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
+    return Error::success();
+  }
+
+  Error VisitObjCObjectPointerTypeLoc(ObjCObjectPointerTypeLoc From) {
+    auto To = ToL.castAs<ObjCObjectPointerTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
+    return Error::success();
+  }
+
+  Error VisitLValueReferenceTypeLoc(LValueReferenceTypeLoc From) {
+    auto To = ToL.castAs<LValueReferenceTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(AmpLoc);
+    return Error::success();
+  }
+
+  Error VisitRValueReferenceTypeLoc(RValueReferenceTypeLoc From) {
+    auto To = ToL.castAs<RValueReferenceTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(AmpAmpLoc);
+    return Error::success();
+  }
+
+  Error VisitFunctionTypeLoc(FunctionTypeLoc From) {
+    auto To = ToL.castAs<FunctionTypeLoc>();
+
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LocalRangeBegin);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LocalRangeEnd);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LParenLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(RParenLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(ExceptionSpecRange);
+
+    for (unsigned I = 0; I < From.getNumParams(); ++I)
+      To.setParam(
+          I, cast_or_null<ParmVarDecl>(
+                 // In the case of importing a lambda CXXRecordDecl the
+                 // parameter had not been imported yet. That will be imported
+                 // later when the lambda's operator() is imported.
+                 Importer.GetAlreadyImportedOrNull(From.getParam(I))));
+
+    return Error::success();
+  }
+
+  Error VisitArrayTypeLoc(ArrayTypeLoc From) {
+    auto To = ToL.castAs<ArrayTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LBracketLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(RBracketLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(SizeExpr);
+    return Error::success();
+  }
+
+  Error VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc From) {
+    auto To = ToL.castAs<TemplateSpecializationTypeLoc>();
+
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(TemplateKeywordLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LAngleLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(RAngleLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(TemplateNameLoc);
+
+    ASTNodeImporter NodeImporter(Importer);
+    for (unsigned I = 0; I < From.getNumArgs(); ++I) {
+      if (Expected<TemplateArgumentLoc> TAL =
+              NodeImporter.import(From.getArgLoc(I)))
+        To.setArgLocInfo(I, TAL->getLocInfo());
+      else
+        return TAL.takeError();
+    }
+
+    return Error::success();
+  }
+
+  Error VisitDependentAddressSpaceTypeLoc(DependentAddressSpaceTypeLoc From) {
+    auto To = ToL.castAs<DependentAddressSpaceTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(AttrNameLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(AttrOperandParensRange);
+    // FIXME: Import other things?
+    return Error::success();
+  }
+
+  Error VisitTypeOfTypeLoc(TypeOfTypeLoc From) {
+    auto To = ToL.castAs<TypeOfTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(UnderlyingTInfo);
+    return Error::success();
+  }
+
+  Error VisitUnaryTransformTypeLoc(UnaryTransformTypeLoc From) {
+    auto To = ToL.castAs<UnaryTransformTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(KWLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LParenLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(RParenLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(UnderlyingTInfo);
+    return Error::success();
+  }
+
+  Error VisitDeducedTemplateSpecializationTypeLoc(
+      DeducedTemplateSpecializationTypeLoc From) {
+    auto To = ToL.castAs<DeducedTemplateSpecializationTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(TemplateNameLoc);
+    return Error::success();
+  }
+
+  Error VisitElaboratedTypeLoc(ElaboratedTypeLoc From) {
+    auto To = ToL.castAs<ElaboratedTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(ElaboratedKeywordLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(QualifierLoc);
+    return Error::success();
+  }
+
+  Error VisitDependentNameTypeLoc(DependentNameTypeLoc From) {
+    auto To = ToL.castAs<DependentNameTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(ElaboratedKeywordLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(QualifierLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(NameLoc);
+    return Error::success();
+  }
+
+  Error VisitDependentTemplateSpecializationTypeLoc(
+      DependentTemplateSpecializationTypeLoc From) {
+    auto To = ToL.castAs<DependentTemplateSpecializationTypeLoc>();
+
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(ElaboratedKeywordLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(QualifierLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(TemplateKeywordLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(TemplateNameLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LAngleLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(RAngleLoc);
+
+    ASTNodeImporter NodeImporter(Importer);
+    for (unsigned I = 0; I < From.getNumArgs(); ++I) {
+      if (Expected<TemplateArgumentLoc> TAL =
+              NodeImporter.import(From.getArgLoc(I)))
+        To.setArgLocInfo(I, TAL->getLocInfo());
+      else
+        return TAL.takeError();
+    }
+
+    return Error::success();
+  }
+
+  Error VisitPackExpansionTypeLoc(PackExpansionTypeLoc From) {
+    auto To = ToL.castAs<PackExpansionTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(EllipsisLoc);
+    return Error::success();
+  }
+
+  Error VisitAtomicTypeLoc(AtomicTypeLoc From) {
+    auto To = ToL.castAs<AtomicTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(KWLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(LParenLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(RParenLoc);
+    return Error::success();
+  }
+
+  Error VisitPipeTypeLoc(PipeTypeLoc From) {
+    auto To = ToL.castAs<PipeTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(KWLoc);
+    return Error::success();
+  }
+
+  Error VisitObjCTypeParamTypeLoc(ObjCTypeParamTypeLoc From) {
+    auto To = ToL.castAs<ObjCTypeParamTypeLoc>();
+    if (From.getNumProtocols()) {
+      IMPORT_TYPE_LOC_OR_RETURN_ERROR(ProtocolLAngleLoc);
+      IMPORT_TYPE_LOC_OR_RETURN_ERROR(ProtocolRAngleLoc);
+      for (unsigned I = 0; I < From.getNumProtocols(); ++I) {
+        if (ExpectedSLoc L = Importer.Import(From.getProtocolLoc(I)))
+          To.setProtocolLoc(I, *L);
+        else
+          return L.takeError();
+      }
+    }
+    return Error::success();
+  }
+
+  Error VisitObjCObjectTypeLoc(ObjCObjectTypeLoc From) {
+    auto To = ToL.castAs<ObjCObjectTypeLoc>();
+
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(TypeArgsLAngleLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(TypeArgsRAngleLoc);
+
+    for (unsigned I = 0; I < From.getNumTypeArgs(); ++I) {
+      if (Expected<TypeSourceInfo *> TSI =
+              Importer.Import(From.getTypeArgTInfo(I)))
+        To.setTypeArgTInfo(I, *TSI);
+      else
+        return TSI.takeError();
+    }
+
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(ProtocolLAngleLoc);
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(ProtocolRAngleLoc);
+
+    for (unsigned I = 0; I < From.getNumProtocols(); ++I) {
+      if (ExpectedSLoc L = Importer.Import(From.getProtocolLoc(I)))
+        To.setProtocolLoc(I, *L);
+      else
+        return L.takeError();
+    }
+
+    To.setHasBaseTypeAsWritten(From.hasBaseTypeAsWritten());
+
+    return Error::success();
+  }
+
+  Error VisitObjCInterfaceTypeLoc(ObjCInterfaceTypeLoc From) {
+    auto To = ToL.castAs<ObjCInterfaceTypeLoc>();
+    IMPORT_TYPE_LOC_OR_RETURN_ERROR(NameLoc);
+    return Error::success();
+  }
+
+};
+
+#undef IMPORT_TYPE_LOC_OR_RETURN_ERROR
+
+} // namespace clang
+
 Expected<TypeSourceInfo *> ASTImporter::Import(TypeSourceInfo *FromTSI) {
   if (!FromTSI)
     return FromTSI;
 
-  // FIXME: For now we just create a "trivial" type source info based
-  // on the type and a single location. Implement a real version of this.
   ExpectedType TOrErr = Import(FromTSI->getType());
   if (!TOrErr)
     return TOrErr.takeError();
-  ExpectedSLoc BeginLocOrErr = Import(FromTSI->getTypeLoc().getBeginLoc());
-  if (!BeginLocOrErr)
-    return BeginLocOrErr.takeError();
 
-  return ToContext.getTrivialTypeSourceInfo(*TOrErr, *BeginLocOrErr);
+  TypeSourceInfo *ToTSI = ToContext.CreateTypeSourceInfo(*TOrErr);
+
+  TypeLoc FromL = FromTSI->getTypeLoc();
+  TypeLoc ToL = ToTSI->getTypeLoc();
+  while (FromL) {
+    assert(ToL && "Not consistent TypeSourceInfo");
+    TypeLocImporter Importer(*this, ToL);
+    if (Error Err = Importer.Visit(FromL))
+      return std::move(Err);
+    FromL = FromL.getNextTypeLoc();
+    ToL = ToL.getNextTypeLoc();
+  }
+
+  return ToTSI;
 }
 
 Expected<Attr *> ASTImporter::Import(const Attr *FromAttr) {
   Attr *ToAttr = FromAttr->clone(ToContext);
   if (auto ToRangeOrErr = Import(FromAttr->getRange()))
     ToAttr->setRange(*ToRangeOrErr);
   else
     return ToRangeOrErr.takeError();

clang/unittests/AST/ASTImporterTest.cpp

TEST_P(ImportAutoFunctions, ReturnWithTypeInSwitch) {
   FunctionDecl *From = FirstDeclMatcher<FunctionDecl>().match(
       FromTU, functionDecl(hasName("f")));
 
   FunctionDecl *To = Import(From, Lang_CXX17);
   EXPECT_TRUE(To);
   EXPECT_TRUE(isa<AutoType>(To->getReturnType()));
 }
 
+struct ImportTypeLoc : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportTypeLoc, Function) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      int f(int p) {
+        return 1;
+      }
+      )",
+      Lang_CXX11, "input0.cc");
+  FunctionDecl *FromF = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f")));
+  FunctionDecl *ToF = Import(FromF, Lang_CXX11);
+  ASSERT_TRUE(ToF);
+  FunctionProtoTypeLoc TL =
+      ToF->getTypeSourceInfo()->getTypeLoc().castAs<FunctionProtoTypeLoc>();
+  EXPECT_EQ(TL.getNumParams(), 1u);
+  EXPECT_TRUE(TL.getParam(0));
+}
+
+TEST_P(ImportTypeLoc, Lambda) {
+  // In this test case, first the CXXRecordDecl of the lambda is imported, then
+  // the operator().
+  Decl *FromTU = getTuDecl(
+      R"(
+      auto l1 = [](unsigned lp) { return 1; };
+      int f(int p) {
+        return l1(p);
+      }
+      )",
+      Lang_CXX11, "input0.cc");
+  FunctionDecl *FromF = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f")));
+  FunctionDecl *ToF = Import(FromF, Lang_CXX11);
+  ASSERT_TRUE(ToF);
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  CXXMethodDecl *ToLambdaMD = FirstDeclMatcher<LambdaExpr>()
+                                  .match(ToTU, lambdaExpr())
+                                  ->getCallOperator();
+  FunctionProtoTypeLoc TL = ToLambdaMD->getTypeSourceInfo()
+                                ->getTypeLoc()
+                                .castAs<FunctionProtoTypeLoc>();
+  EXPECT_EQ(TL.getNumParams(), 1u);
+  EXPECT_TRUE(TL.getParam(0));
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
                         DefaultTestValuesForRunOptions, );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportPath,
                         ::testing::Values(ArgVector()), );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportExpr,
                         DefaultTestValuesForRunOptions, );
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods,
                         DefaultTestValuesForRunOptions, );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables,
                         DefaultTestValuesForRunOptions, );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest,
                         DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportTypeLoc,
+                        DefaultTestValuesForRunOptions, );
+
 } // end namespace ast_matchers
 } // end namespace clang