[ASTImporter] import FunctionDecl end locations

[r.stahl]

Summary

On constructors that do not take the end source location, it was not imported. Fixes test from D47698 / rC336269.

[r.stahl]

In D47698#1141871, @r.stahl wrote:

improved code quality; added nested macro test. it "works", but is disabled because it revealed another bug: the function end location is not imported. will send a patch

Related to this.

[martong]

lib/AST/ASTImporter.cpp

2549

Why don't we need to call the setRangeEnd() when e.g. we create a CXXMethodDecl or a CXXConversionDecl? Couldn't we just call this after all these branches (at line 2587)?

[a_sidorin]

Hello Rafael.
This change is good, just some cleanup is needed.

lib/AST/ASTImporter.cpp

2549

I guess the reason is that they take LocEnd as ctor arguments. However, I think that having a single point is better.

[r.stahl]

Alright, but then I would suggest to pass an invalid loc to the constructors instead to make it more explicit and save a map lookup in the import function.

[martong]

LGTM!

[a_sidorin]

LGTM too. Thank you!

lib/AST/ASTImporter.cpp

if (auto *FromConstructor = dyn_cast<CXXConstructorDecl>(D)) {
     }
   } else if (isa<CXXDestructorDecl>(D)) {
     ToFunction = CXXDestructorDecl::Create(Importer.getToContext(),
                                            cast<CXXRecordDecl>(DC),
                                            InnerLocStart,
                                            NameInfo, T, TInfo,
                                            D->isInlineSpecified(),
                                            D->isImplicit());
   } else if (auto *FromConversion = dyn_cast<CXXConversionDecl>(D)) {
     ToFunction = CXXConversionDecl::Create(Importer.getToContext(), 
                                            cast<CXXRecordDecl>(DC),
                                            InnerLocStart,
                                            NameInfo, T, TInfo,
                                            D->isInlineSpecified(),
                                            FromConversion->isExplicit(),
                                            D->isConstexpr(),
-                                           Importer.Import(D->getLocEnd()));
+                                           SourceLocation());
   } else if (auto *Method = dyn_cast<CXXMethodDecl>(D)) {
     ToFunction = CXXMethodDecl::Create(Importer.getToContext(), 
                                        cast<CXXRecordDecl>(DC),
                                        InnerLocStart,
                                        NameInfo, T, TInfo,
                                        Method->getStorageClass(),
                                        Method->isInlineSpecified(),
                                        D->isConstexpr(),
-                                       Importer.Import(D->getLocEnd()));
+                                       SourceLocation());
   } else {
     ToFunction = FunctionDecl::Create(Importer.getToContext(), DC,
                                       InnerLocStart,
                                       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());
   ToFunction->setLexicalDeclContext(LexicalDC);
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
   Importer.Imported(D, ToFunction);
 
   // Set the parameters.
   for (auto *Param : Parameters) {
     Param->setOwningFunction(ToFunction);
     ToFunction->addDeclInternal(Param);
   }
   ToFunction->setParams(Parameters);

unittests/AST/ASTImporterTest.cpp

TEST_P(ASTImporterTestBase, ImportSourceLocs) {
   CompareSourceRanges(ToD->getSourceRange(), FromD->getSourceRange(), ToSM,
                       FromSM);
   CompareSourceRanges(ToLHS->getSourceRange(), FromLHS->getSourceRange(), ToSM,
                       FromSM);
   CompareSourceRanges(ToRHS->getSourceRange(), FromRHS->getSourceRange(), ToSM,
                       FromSM);
 }
 
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
   Decl *FromTU = getTuDecl(
       R"(
       #define FUNC_INT void declToImport
       #define FUNC FUNC_INT
       FUNC(int a);
       )",
       Lang_CXX);
   auto FromD = FirstDeclMatcher<FunctionDecl>().match(FromTU, functionDecl());