[PATCH] D75048: [ASTImporter] Improved import of AlignedAttr.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

LGTM aside from the comments I made.



================
Comment at: clang/lib/AST/ASTImporter.cpp:7944
+      if (auto ToEOrErr = Import(From->getAlignmentExpr()))
+        To = AlignedAttr::Create(ToContext, true, *ToEOrErr, ToRange,
+                                 FromAttr->getSyntax(),
----------------
This call to `Create` and the one below look identical can we please refactor to avoid code duplication.


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5891
+TEST_P(ASTImporterOptionSpecificTestBase, ImportExprOfAlignmentAttr) {
+  // FIXME: These packed and aligned attributes could trigger an error situation
+  // where source location from 'From' context is referenced in 'To' context
----------------
If we have a test case that triggers this failure can we add the test as a know failing test and then we the FIXME is done the test should pass.


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5910
+  auto *ToAttr = ToD->getAttr<AlignedAttr>();
+  EXPECT_EQ(FromAttr->isInherited(), ToAttr->isInherited());
+  EXPECT_EQ(FromAttr->isPackExpansion(), ToAttr->isPackExpansion());
----------------
What about `getSemanticSpelling()`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75048/new/

https://reviews.llvm.org/D75048