[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D75574#2262622 <https://reviews.llvm.org/D75574#2262622>, @lanza wrote:

>> I don't think it'll actually error out at link time: protocol objects get emitted eagerly on use, cross-module linking is just a code-size optimization. This actually has caused longstanding problems.
>
> But if it's just a forward declaration there's nothing to emit. The above code compiles just fine as is with just a warning. Here's the result of `clang protocol.m -lobjc`
>
>   proto.m:10:31: warning: cannot find protocol definition for 'NonRuntimeProto'
>   @interface Implementer : Root<NonRuntimeProto>
>                                 ^
>   proto.m:8:11: note: protocol 'NonRuntimeProto' has no definition
>   @protocol NonRuntimeProto;
>             ^
>   1 warning generated.
>   Undefined symbols for architecture x86_64:
>     "__OBJC_PROTOCOL_$_NonRuntimeProto", referenced from:
>         __OBJC_CLASS_PROTOCOLS_$_Implementer in proto-bd4a43.o
>   ld: symbol(s) not found for architecture x86_64
>   clang-12: error: linker command failed with exit code 1 (use -v to see invocation)
>
> The protocol definition isn't actually required to compile an implementation. And if that protocol is declared as `objc_non_runtime_protocol` it won't ever see one.
>
> Simply requiring that it is annotated accordingly also isn't satisfactory for the same inheritance problem you mentioned above. Annotating a forward decl can tell clang not to emit it but won't let clang know if there's a base protocol that still needs to be emitted. e.g. if we have
>
>   // SomeHeader.h
>   @protocol Base
>   @end
>   __attribute__((objc_non_runtime_protocol))
>   @protocol NonRuntime <Base>
>   @end
>   
>   
>   // and in main.m
>   __attribute__((objc_non_runtime_protocol))
>   @protocol NonRuntime
>   @interface AClass : Root<NonRunime>
>   @end
>   @implementation AClass
>   @end
>
> we'll get a compile warning but no link error. But it will be wrong as the protocol `Base` should still be in the protocollist for `AClass`.
>
> I'm not sure how big of an issue it would be introducing a new error here for code that used to compile, but that's really the only way I see this working.

Hmm, I thought we actually just generated a bogus definition for the protocol when it was forward-declared; really, this is better behavior that I expected.  Regardless, I don't think it's worthwhile to diagnose this more strongly than a warning because of the history of not doing so.

Not really important for this PR anyway.  Was your question answered well enough to move forward?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75574/new/

https://reviews.llvm.org/D75574