Page MenuHomePhabricator

[clang] Replace Decl::isUnconditionallyVisible() with Sema::isVisible()
Needs ReviewPublic

Authored by mboehme on Fri, Jun 12, 5:10 AM.

Details

Reviewers
rsmith
Summary

For context, see
https://bugs.llvm.org/show_bug.cgi?id=46248

This handles only the easy cases in Sema/SemaDeclObjC.cpp. The cases in AST/DeclObjC.{h,cpp} will require much more work, but there's no reason not to get the easy work out of the way now.

Diff Detail

Event Timeline

mboehme created this revision.Fri, Jun 12, 5:10 AM

Test case(s)?

mboehme added a comment.EditedMon, Jun 15, 9:51 PM

Test case(s)?

Is there anything specific you have in mind?

This change should be behavior-preserving in the case where -fmodules-local-submodule-visibility isn't set, as evidenced by the existing tests, which continue to pass.

This change is a small step towards making -fmodules-local-submodule-visibility work for Objective-C, but unfortunately we're still a long way off from being able to turn this flag on; activating it for existing tests still causes them to fail because parts of AST are still using Decl::isUnconditionallyVisible() instead of Sema::isVisible(), and that's unfortunately harder to fix.

Test case(s)?

Is there anything specific you have in mind?

This change should be behavior-preserving in the case where -fmodules-local-submodule-visibility isn't set, as evidenced by the existing tests, which continue to pass.

This change is a small step towards making -fmodules-local-submodule-visibility work for Objective-C, but unfortunately we're still a long way off from being able to turn this flag on; activating it for existing tests still causes them to fail because parts of AST are still using Decl::isUnconditionallyVisible() instead of Sema::isVisible(), and that's unfortunately harder to fix.

Maybe take the smallest/simplest example using -fmodules-local-submodule-visibility and Objective-C and add all Sema::isVisible needed to make that work & include that as a test?