[ASTImporter] Fix import of a typedef that has an attribute

[martong]

Summary

The import of a typedefs with an attribute uses clang::Decl::setAttrs().
But that needs the ASTContext which we can get only from the
TranslationUnitDecl. But we can get the TUDecl only thourgh the
DeclContext, which is not set by the time of the setAttrs call.

Fix: import the attributes only after the DC is surely imported.
Btw, having the attribute import initiated from GetImportedOrCreateDecl was
fundamentally flawed. Now that is implicitly fixed.

[shafik]

LGTM

clang/unittests/AST/ASTImporterTest.cpp

6098

We can't check the attribute is present?

[martong]

• Check the attribute in the test as well

[martong]

clang/unittests/AST/ASTImporterTest.cpp

6098

Yes, good point. I've update with some more checks. Thanks for the review!

[martong]

Shafik, thanks for the review!

clang/lib/AST/ASTImporter.cpp

GetImportedOrCreateSpecialDecl(ToDeclT *&ToD, CreateFunT CreateFun,
       // Keep track of imported Decls.
       Importer.RegisterImportedDecl(FromD, ToD);
       InitializeImportedDecl(FromD, ToD);
       return false; // A new Decl is created.
     }
 
     void InitializeImportedDecl(Decl *FromD, Decl *ToD) {
       ToD->IdentifierNamespace = FromD->IdentifierNamespace;
-      if (FromD->hasAttrs())
-        for (const Attr *FromAttr : FromD->getAttrs()) {
-          // FIXME: Return of the error here is not possible until store of
-          // import errors is implemented.
-          auto ToAttrOrErr = import(FromAttr);
-          if (ToAttrOrErr)
-            ToD->addAttr(*ToAttrOrErr);
-          else
-            llvm::consumeError(ToAttrOrErr.takeError());
-        }
       if (FromD->isUsed())
         ToD->setIsUsed();
       if (FromD->isImplicit())
         ToD->setImplicit();
     }
 
     // Check if we have found an existing definition.  Returns with that
     // definition if yes, otherwise returns null.
Expected<Decl *> ASTImporter::Import(Decl *FromD) {
   if (auto Error = SharedState->getImportDeclErrorIfAny(ToD)) {
     setImportDeclError(FromD, *Error);
     return make_error<ImportError>(*Error);
   }
 
   // Make sure that ImportImpl registered the imported decl.
   assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
 
+  if (FromD->hasAttrs())
+    for (const Attr *FromAttr : FromD->getAttrs()) {
+      auto ToAttrOrErr = Import(FromAttr);
+      if (ToAttrOrErr)
+        ToD->addAttr(*ToAttrOrErr);
+      else
+        return ToAttrOrErr.takeError();
+    }
+
   // Notify subclasses.
   Imported(FromD, ToD);
 
   updateFlags(FromD, ToD);
   SavedImportPaths.erase(FromD);
   return ToDOrErr;
 }
 

clang/unittests/AST/ASTImporterTest.cpp

auto *Typedef = FirstDeclMatcher<TypedefNameDecl>().match(
       TU, typedefNameDecl(hasName("U")));
   ParmVarDecl *Param = Guide->getParamDecl(0);
   EXPECT_NE(Param->getType()->getAs<TypedefType>()->getDecl(), Typedef);
 }
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, CTAD,
                         DefaultTestValuesForRunOptions, );
 
+TEST_P(ASTImporterOptionSpecificTestBase, TypedefWithAttribute) {
+  Decl *TU = getTuDecl(
+      R"(
+      namespace N {
+        typedef int X __attribute__((annotate("A")));
+      }
+      )",
+      Lang_CXX17, "input.cc");
+  auto *FromD =
+      FirstDeclMatcher<TypedefDecl>().match(TU, typedefDecl(hasName("X")));
+  auto *ToD = Import(FromD, Lang_CXX17);
+  ASSERT_TRUE(ToD);
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
                         DefaultTestValuesForRunOptions, );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportPath,
                         ::testing::Values(std::vector<std::string>()), );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportExpr,
                         DefaultTestValuesForRunOptions, );