This is an archive of the discontinued LLVM Phabricator instance.

[clang] Remove that ASTContext::getObjCInterfaceType eagerly pulls in definitions of ObjCInterfaceDecls
AcceptedPublic

Authored by teemperor on Nov 3 2021, 6:52 AM.

Details

Reviewers
vsapsai
Summary

ASTContext::getObjCInterfaceType always tries to find the definition of the ObjCInterfaceDecl which
we try to create a type for. This code doesn't seem to serve any purpose from what I can see.

I checked all Clang tests and compiled all LLDB Foundation tests with -fmodules and I couldn't find a
single call where this code has any effect (beside spending a bit of time looking for a definition). We
always either don't have a definition at all or the current Decl is already the definition so nothing changes.

The code also can't save us any time as the way we retrieve the Decl later is by first unconditionally
throwing away the definition via getCanonicalDecl() and then we try to find the definition again:

ObjCInterfaceDecl *ObjCInterfaceType::getDecl() const {
  ObjCInterfaceDecl *Canon = Decl->getCanonicalDecl();
  if (ObjCInterfaceDecl *Def = Canon->getDefinition())
    return Def;
  return Canon;
}

From the git log it seems like this was some (pre-modules) attempt to make sure we always point to
the definition from an ObjCInterfaceType. That code was since updated to work with modules in
D110452 (which added the new version of getDecl that correctly updates the redeclarations
to find a definition from a module).

Diff Detail

Event Timeline

teemperor requested review of this revision.Nov 3 2021, 6:52 AM
teemperor created this revision.
vsapsai accepted this revision.Nov 3 2021, 2:23 PM

Looks good to me. If avoiding finding a definition at this point helps you - perfect.

The intention behind D110452 was that ObjCInterfaceType shouldn't carefully track the correct definition, it should rely on the redeclaration chain already tracking the correct definition. And your change fits into this approach.

This revision is now accepted and ready to land.Nov 3 2021, 2:23 PM