This is an archive of the discontinued LLVM Phabricator instance.

[ASTImporter] Improved import of AlignedAttr.
ClosedPublic

Authored by balazske on Feb 24 2020, 6:36 AM.

Details

Summary

It is not enough to clone the attributes at import.
They can contain reference to objects that should be imported.
This work is done now for AlignedAttr.

Event Timeline

balazske created this revision.Feb 24 2020, 6:36 AM
Herald added a reviewer: shafik. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
shafik accepted this revision.Feb 24 2020, 12:54 PM

LGTM aside from the comments I made.

clang/lib/AST/ASTImporter.cpp
7944

This call to Create and the one below look identical can we please refactor to avoid code duplication.

clang/unittests/AST/ASTImporterTest.cpp
5891

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.

5910

What about getSemanticSpelling()?

This revision is now accepted and ready to land.Feb 24 2020, 12:54 PM
balazske updated this revision to Diff 246392.Feb 25 2020, 2:27 AM
  • Do not call AlignedAttr::Create with same parameters at 2 places. I think the code duplication was near to each other and only 2 instances that is not a big problem, anyway now it is a bit better.
  • Improved the comment in the test and added some new checks.
balazske marked 3 inline comments as done.Feb 25 2020, 2:27 AM
shafik added inline comments.Feb 25 2020, 1:11 PM
clang/lib/AST/ASTImporter.cpp
7944

How about something more like, maybe I am missing a detail but hopefully not:

bool IsAlignmentExpr=From->isAlignmentExpr();
auto ToEOrErr = [IsAlignmentExpr]() {
  if (IsAlignmentExpr)
    return Import(From->getAlignmentExpr());

  return  Import(From->getAlignmentType());
}();

if (!ToTOrErr)
  return ToTOrErr.takeError();

To = AlignedAttr::Create(ToContext, IsAlignmentExpr, *ToEOrErr, ToRange,
                                 From->getSyntax(),From->getSemanticSpelling());
balazske marked an inline comment as done.Feb 26 2020, 12:19 AM
balazske added inline comments.
clang/lib/AST/ASTImporter.cpp
7944

It is only possible by using a template. Importing of AlignmentExpr returns Expected<Expr *> and importing the AlignmentType returns Expected<TypeSourceInfo *>. The ToTOrErr and ToEOrErr are of different types. (The create function expects a void * that can be a pointer to Expr or TypeSourceInfo).

This revision was automatically updated to reflect the committed changes.