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

2153

This chunk should be fixed by r236012.

2863

Still actual.

6135

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->parameters()) {
     ParmVarDecl *ToP = cast_or_null<ParmVarDecl>(Importer.Import(P));
     if (!ToP)
       return nullptr;
 
     Parameters.push_back(ToP);
   }
   
   // Create the imported function.
   TypeSourceInfo *TInfo = Importer.Import(D->getTypeSourceInfo());
   FunctionDecl *ToFunction = nullptr;
   SourceLocation InnerLocStart = Importer.Import(D->getInnerLocStart());
   if (CXXConstructorDecl *FromConstructor = dyn_cast<CXXConstructorDecl>(D)) {
     ToFunction = CXXConstructorDecl::Create(Importer.getToContext(),
                                             cast<CXXRecordDecl>(DC),
                                             InnerLocStart,
                                             NameInfo, T, TInfo, 
                                             FromConstructor->isExplicit(),
for (auto *FromP : D->parameters()) {
     ToParams.push_back(ToP);
   }
   
   // 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);
-  ToMethod->setMethodParams(Importer.getToContext(), ToParams, 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;
 }
 
 Decl *ASTNodeImporter::VisitObjCTypeParamDecl(ObjCTypeParamDecl *D) {
TypeSourceInfo *ASTImporter::Import(TypeSourceInfo *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, 
            Import(FromTSI->getTypeLoc().getLocStart()));
 }
 
 Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
   llvm::DenseMap<Decl *, Decl *>::iterator Pos = ImportedDecls.find(FromD);
   if (Pos != ImportedDecls.end()) {
     Decl *ToD = Pos->second;
     ASTNodeImporter(*this).ImportDefinitionIfNeeded(FromD, ToD);
     return ToD;
   } else {

test/ASTMerge/interface/Inputs/interface1.m

 
 // Implementation by itself
 @implementation I14 : I12
 @end
 
 @implementation I15 : I12
 @end
 
-
+@interface ImportSelectorSLoc { }
+-(int)addInt:(int)a toInt:(int)b moduloInt:(int)c; // don't crash here
+@end