Page MenuHomePhabricator

[ASTImporter] Support importing CXXPseudoDestructorExpr

Authored by szepet on Oct 12 2017, 5:14 AM.

Diff Detail

Event Timeline

szepet created this revision.Oct 12 2017, 5:14 AM
xazax.hun added inline comments.Oct 19 2017, 2:36 AM

Does E->getBase() guaranteed to return non-null? What happens when this node was constructed using EmptyShell? Shouldn't we check for that somehow? When can that happen?

szepet added inline comments.Oct 19 2017, 6:25 AM

The import process of ArraySubscriptExpr and UnaryOperator (and probably more other classes) are not prepared for this case as well. Not sure if this can be encountered in a complete AST.
However, I think a lazy evaluated && operator won't hurt the performance and at least we are going to be prepared for this case.

szepet updated this revision to Diff 119579.Oct 19 2017, 6:26 AM

Checking for importing base updated to (!Imported && From) style.

a.sidorin edited edge metadata.Nov 13 2017, 9:20 AM

Hello Peter,

Patch is almost OK but there are some minor issues.

Expr *getBase() const { return cast<Expr>(Base); }

This means it is always non-null, I guess.


Source ScopeInfo can be null but we still need to check the result.

  1. Needs to be aligned.
  2. Unused templates are common source of test failures on Windows (MSVC). Could you add a template instantiation (or check it works on Windows as expected)?
szepet updated this revision to Diff 124173.Nov 24 2017, 5:22 AM
szepet marked 3 inline comments as done.

Thank you for the review!

Updated according to review comments.

a.sidorin accepted this revision.Nov 26 2017, 5:19 AM

Looks good, thank you!

This revision is now accepted and ready to land.Nov 26 2017, 5:19 AM
This revision was automatically updated to reflect the committed changes.