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

[mboehme]

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.

[dblaikie]

Test case(s)?

[mboehme]

In D81732#2093260, @dblaikie wrote:

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.

[dblaikie]

In D81732#2094735, @mboehme wrote:

In D81732#2093260, @dblaikie wrote:

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?

clang/lib/Sema/SemaDeclObjC.cpp

if (!err && NumProtoRefs ) {
     PDecl->setProtocolList((ObjCProtocolDecl*const*)ProtoRefs, NumProtoRefs,
                            ProtoLocs, Context);
   }
 
   CheckObjCDeclScope(PDecl);
   return ActOnObjCContainerStartDefinition(PDecl);
 }
 
-static bool NestedProtocolHasNoDefinition(ObjCProtocolDecl *PDecl,
-                                          ObjCProtocolDecl *&UndefinedProtocol) {
-  if (!PDecl->hasDefinition() ||
-      !PDecl->getDefinition()->isUnconditionallyVisible()) {
+static bool
+NestedProtocolHasNoDefinition(Sema &SemaRef, ObjCProtocolDecl *PDecl,
+  if (!PDecl->hasDefinition() || !SemaRef.isVisible(PDecl->getDefinition())) {
     UndefinedProtocol = PDecl;
     return true;
   }
 
   for (auto *PI : PDecl->protocols())
-    if (NestedProtocolHasNoDefinition(PI, UndefinedProtocol)) {
+    if (NestedProtocolHasNoDefinition(SemaRef, PI, UndefinedProtocol)) {
       UndefinedProtocol = PI;
       return true;
     }
   return false;
 }
 
 /// FindProtocolDeclaration - This routine looks up protocols and
 /// issues an error if they are not declared. It returns list of
for (const IdentifierLocPair &Pair : ProtocolId) {
     }
 
     // If this is a forward declaration and we are supposed to warn in this
     // case, do it.
     // FIXME: Recover nicely in the hidden case.
     ObjCProtocolDecl *UndefinedProtocol;
 
     if (WarnOnDeclarations &&
-        NestedProtocolHasNoDefinition(PDecl, UndefinedProtocol)) {
+        NestedProtocolHasNoDefinition(*this, PDecl, UndefinedProtocol)) {
       Diag(Pair.second, diag::warn_undef_protocolref) << Pair.first;
       Diag(UndefinedProtocol->getLocation(), diag::note_protocol_decl_undefined)
         << UndefinedProtocol;
     }
     Protocols.push_back(PDecl);
   }
 }
 
for (unsigned i = 0, n = protocols.size(); i != n; ++i) {
       if (!proto->isThisDeclarationADefinition() && proto->getDefinition())
         proto = proto->getDefinition();
 
       // If this is a forward declaration and we are supposed to warn in this
       // case, do it.
       // FIXME: Recover nicely in the hidden case.
       ObjCProtocolDecl *forwardDecl = nullptr;
       if (warnOnIncompleteProtocols &&
-          NestedProtocolHasNoDefinition(proto, forwardDecl)) {
+          NestedProtocolHasNoDefinition(*this, proto, forwardDecl)) {
         Diag(identifierLocs[i], diag::warn_undef_protocolref)
           << proto->getDeclName();
         Diag(forwardDecl->getLocation(), diag::note_protocol_decl_undefined)
           << forwardDecl;
       }
 
       // If everything this far has been a type name (and we care
       // about such things), check whether this name refers to a type

 bool Sema::MatchTwoMethodDeclarations(const ObjCMethodDecl *left,
                                       const ObjCMethodDecl *right,
                                       MethodMatchStrategy strategy) {
   if (!matchTypes(Context, strategy, left->getReturnType(),
                   right->getReturnType()))
     return false;
 
   // If either is hidden, it is not considered to match.
-  if (!left->isUnconditionallyVisible() || !right->isUnconditionallyVisible())
+  if (!isVisible(left) || !isVisible(right))
     return false;
 
   if (left->isDirectMethod() != right->isDirectMethod())
     return false;
 
   if (getLangOpts().ObjCAutoRefCount &&
       (left->hasAttr<NSReturnsRetainedAttr>()
          != right->hasAttr<NSReturnsRetainedAttr>() ||
bool Sema::CollectMultipleMethodsInGlobalPool(
   GlobalMethodPool::iterator Pos = MethodPool.find(Sel);
   if (Pos == MethodPool.end())
     return false;
 
   // Gather the non-hidden methods.
   ObjCMethodList &MethList = InstanceFirst ? Pos->second.first :
                              Pos->second.second;
   for (ObjCMethodList *M = &MethList; M; M = M->getNext())
-    if (M->getMethod() && M->getMethod()->isUnconditionallyVisible()) {
+    if (M->getMethod() && isVisible(M->getMethod())) {
       if (FilterMethodsByTypeBound(M->getMethod(), TypeBound))
         Methods.push_back(M->getMethod());
     }
 
   // Return if we find any method with the desired kind.
   if (!Methods.empty())
     return Methods.size() > 1;
 
   if (!CheckTheOther)
     return false;
 
   // Gather the other kind.
   ObjCMethodList &MethList2 = InstanceFirst ? Pos->second.second :
                               Pos->second.first;
   for (ObjCMethodList *M = &MethList2; M; M = M->getNext())
-    if (M->getMethod() && M->getMethod()->isUnconditionallyVisible()) {
+    if (M->getMethod() && isVisible(M->getMethod())) {
       if (FilterMethodsByTypeBound(M->getMethod(), TypeBound))
         Methods.push_back(M->getMethod());
     }
 
   return Methods.size() > 1;
 }
 
 bool Sema::AreMultipleMethodsInGlobalPool(
ObjCMethodDecl *Sema::LookupMethodInGlobalPool(Selector Sel, SourceRange R,
   GlobalMethodPool::iterator Pos = MethodPool.find(Sel);
   if (Pos == MethodPool.end())
     return nullptr;
 
   // Gather the non-hidden methods.
   ObjCMethodList &MethList = instance ? Pos->second.first : Pos->second.second;
   SmallVector<ObjCMethodDecl *, 4> Methods;
   for (ObjCMethodList *M = &MethList; M; M = M->getNext()) {
-    if (M->getMethod() && M->getMethod()->isUnconditionallyVisible())
+    if (M->getMethod() && isVisible(M->getMethod()))
       return M->getMethod();
   }
   return nullptr;
 }
 
 void Sema::DiagnoseMultipleMethodInGlobalPool(SmallVectorImpl<ObjCMethodDecl*> &Methods,
                                               Selector Sel, SourceRange R,
                                               bool receiverIdOrClass) {