Page MenuHomePhabricator

[ASTImporter] Added visibility context check for CXXRecordDecl.
ClosedPublic

Authored by balazske on May 23 2019, 6:50 AM.

Details

Summary

ASTImporter makes now difference between classes with same name in different
translation units if these are not visible outside. These classes are not linked
into one decl chain.

Diff Detail

Repository
rL LLVM

Event Timeline

balazske created this revision.May 23 2019, 6:50 AM

Minor comments, I am going to run check-lldb now.

unittests/AST/ASTImporterVisibilityTest.cpp
34 ↗(On Diff #200962)

GetCXXRecordPattern feels more consistent.

49 ↗(On Diff #200962)

const? It is not consistent w/ the previous declarations.

50 ↗(On Diff #200962)

const? It is not consistent w/ the previous declarations.

shafik accepted this revision.May 23 2019, 10:13 AM

LGTM

This revision is now accepted and ready to land.May 23 2019, 10:13 AM
balazske marked an inline comment as done.May 24 2019, 12:10 AM

Thanks for reviewing.

Similar additions are planned to follow with function and variable template, enum, scoped enum, typedef, type alias.

unittests/AST/ASTImporterVisibilityTest.cpp
34 ↗(On Diff #200962)

The exact decl name is not repeated in these variable names like "FunPattern" for functions. Using "class" instead of "CXXRecord" can be acceptable. I do not like ImportCXXRecordsVisibilityChain (GetRecPattern and ImportRecordsVisibilityChain may be good too, but we do not say "C++ record" for thing that is a class).

balazske updated this revision to Diff 201133.May 24 2019, 12:58 AM
  • Changed type of string constants to const.
balazske marked 2 inline comments as done.May 24 2019, 1:06 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2019, 2:36 AM