Page MenuHomePhabricator

[modules] Add test for using declaration in classes.
ClosedPublic

Authored by teemperor on Aug 26 2017, 6:31 AM.

Details

Summary

This adds a test that checks if the using declaration in classes still works as intended with modules.

The motivation for this is that we tried to add a shortcut to removeDecl that would skip the removal of declarations from the lookup table if they are hidden. This optimization passed the clang test suite but actually broke the using declaration in combination with -fmodules-local-submodule-visibility. In this mode we hide all decls from other modules such as by chance the parent method, in which case don't remove the parent method from the lookup table and get ambiguous lookup errors. After this patch we now correctly see if this behavior is broken by a patch like this in the test suite.

Diff Detail

Repository
rL LLVM

Event Timeline

teemperor created this revision.Aug 26 2017, 6:31 AM
teemperor edited the summary of this revision. (Show Details)Aug 27 2017, 6:13 AM
teemperor added a reviewer: v.g.vassilev.
teemperor set the repository for this revision to rL LLVM.
teemperor added a subscriber: cfe-commits.
v.g.vassilev accepted this revision.Aug 27 2017, 6:56 AM

LGTM! I'd reword the commit message, saying that we intended to add a fast path in DeclContext::removeDecl which checks if the decl is hidden to avoid addition in the lookup tables. That caused problems with modules, because the modules system extends the definition of visibility and this is a test making sure we do not reintroduce such an issue.

This revision is now accepted and ready to land.Aug 27 2017, 6:56 AM
teemperor edited the summary of this revision. (Show Details)Aug 28 2017, 7:40 AM
This revision was automatically updated to reflect the committed changes.