ASTImporter: Fix missing SourceLoc imports

[a.sidorin]

Summary

Fixes all spots I could identify that use SourceLocs without importing them first.

[nrieck]

Ping.

[a.sidorin]

Hi Gabor. One of the bugs fixed in this patch is still present in master, other two are already fixed.

lib/AST/ASTImporter.cpp

2749

This chunk should be fixed by r236012.

3310

Still actual.

4613

Fixed by r241542.

[xazax.hun]

In D6550#663002, @a.sidorin wrote:

Hi Gabor. One of the bugs fixed in this patch is still present in master, other two are already fixed.

Thanks for checking that! Do you think it is ok for me to commit the missing part?

[a.sidorin]

This patch lacks tests. If you add at least minimal test case (I'm not familiar with ObjC and its front-end, unfortunately), I will no have any concerns. Also adding Sean.

[a.sidorin]

Will anybody object if I commit this change without a test? This bug seems to be pretty obvious but, unfortunately, I'm not familiar with Objective C.

[spyffe]

Hmm, the transforming in place of SelLocs reads a little weirdly to me, but other than that the code seems fine.
Is your concern that you don't know how to write an Objective-C test that would cover this? It looks to me like an Objective-C interface with a method:

@interface MyClass { }
-(int)addInt:(int)a toInt:(int)b moduloInt:(int)c;
@end

might be enough for an ASTMerge test. If you want to make clang-import-test use this code, then we might need to add one or two things into that tester to handle Objective-C method lookup.

[a.sidorin]

Thank you Sean, I'll try.

[a.sidorin]

Removed already fixed stuff, added a test for remaining.

[xazax.hun]

Great! Thanks!

lib/AST/ASTImporter.cpp

for (auto P : D->params()) {
     ParmVarDecl *ToP = cast_or_null<ParmVarDecl>(Importer.Import(P));
     if (!ToP)
       return nullptr;
 
     Parameters.push_back(ToP);
   }
   
   // Create the imported function.
+  SourceLocation StartLoc = Importer.Import(D->getInnerLocStart());
   TypeSourceInfo *TInfo = Importer.Import(D->getTypeSourceInfo());
   FunctionDecl *ToFunction = nullptr;
   if (CXXConstructorDecl *FromConstructor = dyn_cast<CXXConstructorDecl>(D)) {
     ToFunction = CXXConstructorDecl::Create(Importer.getToContext(),
                                             cast<CXXRecordDecl>(DC),
-                                            D->getInnerLocStart(),
-                                            NameInfo, T, TInfo, 
+                                            StartLoc, NameInfo, T, TInfo,
                                             FromConstructor->isExplicit(),
                                             D->isInlineSpecified(), 
                                             D->isImplicit(),
                                             D->isConstexpr());
   } else if (isa<CXXDestructorDecl>(D)) {
     ToFunction = CXXDestructorDecl::Create(Importer.getToContext(),
                                            cast<CXXRecordDecl>(DC),
-                                           D->getInnerLocStart(),
-                                           NameInfo, T, TInfo,
+                                           StartLoc, NameInfo, T, TInfo,
                                            D->isInlineSpecified(),
                                            D->isImplicit());
   } else if (CXXConversionDecl *FromConversion
                                            = dyn_cast<CXXConversionDecl>(D)) {
     ToFunction = CXXConversionDecl::Create(Importer.getToContext(), 
                                            cast<CXXRecordDecl>(DC),
-                                           D->getInnerLocStart(),
-                                           NameInfo, T, TInfo,
+                                           StartLoc, NameInfo, T, TInfo,
                                            D->isInlineSpecified(),
                                            FromConversion->isExplicit(),
                                            D->isConstexpr(),
                                            Importer.Import(D->getLocEnd()));
   } else if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D)) {
     ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
                                        cast<CXXRecordDecl>(DC),
-                                       D->getInnerLocStart(),
-                                       NameInfo, T, TInfo,
+                                       StartLoc, NameInfo, T, TInfo,
                                        Method->getStorageClass(),
                                        Method->isInlineSpecified(),
                                        D->isConstexpr(),
                                        Importer.Import(D->getLocEnd()));
   } else {
     ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
-                                      D->getInnerLocStart(),
-                                      NameInfo, T, TInfo, D->getStorageClass(),
+                                      StartLoc, NameInfo, T, TInfo,
+                                      D->getStorageClass(),
                                       D->isInlineSpecified(),
                                       D->hasWrittenPrototype(),
                                       D->isConstexpr());
   }
 
   // Import the qualifier, if any.
   ToFunction->setQualifierInfo(Importer.Import(D->getQualifierLoc()));
   ToFunction->setAccess(D->getAccess());
Decl *ASTNodeImporter::VisitObjCMethodDecl(ObjCMethodDecl *D) {
   
   // Set the parameters.
   for (unsigned I = 0, N = ToParams.size(); I != N; ++I) {
     ToParams[I]->setOwningFunction(ToMethod);
     ToMethod->addDeclInternal(ToParams[I]);
   }
   SmallVector<SourceLocation, 12> SelLocs;
   D->getSelectorLocs(SelLocs);
+  for (SourceLocation &Loc : SelLocs)
+    Loc = Importer.Import(Loc);
   ToMethod->setMethodParams(Importer.getToContext(), ToParams, SelLocs); 
 
   ToMethod->setLexicalDeclContext(LexicalDC);
   Importer.Imported(D, ToMethod);
   LexicalDC->addDeclInternal(ToMethod);
   return ToMethod;
 }
 
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.
   QualType T = Import(FromTSI->getType());
   if (T.isNull())
     return nullptr;
 
-  return ToContext.getTrivialTypeSourceInfo(T, 
-                        FromTSI->getTypeLoc().getLocStart());
+  return ToContext.getTrivialTypeSourceInfo(
+      T, Import(FromTSI->getTypeLoc().getLocStart()));
 }
 
 Decl *ASTImporter::Import(Decl *FromD) {
   if (!FromD)
     return nullptr;
 
   ASTNodeImporter Importer(*this);