[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

This revision was automatically updated to reflect the committed changes. Closed by commit rC329882: ObjCGNU: Fix empty v3 protocols being emitted two fields short (authored by theraven, committed by ). Repository: rC Clang https://reviews.llvm.org/D45305 Files: lib/CodeGen/CGObjCGNU.cpp

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

DHowett-MSFT added a comment. Hi! Is there anything else holding up this patch? Thanks! Repository: rC Clang https://reviews.llvm.org/D45305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

DHowett-MSFT updated this revision to Diff 141701. DHowett-MSFT edited the summary of this revision. DHowett-MSFT added a comment. I've fixed the test to check the LLVM IR; I chose to use `CHECK-SAME` and use indentation to clarify what was going on. We're now checking that an empty protocol is

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

theraven added a comment. I think that we emit empty method lists so that the GCC runtime doesn't choke on them, though there's no reason why we couldn't with the GNUstep ABI. It would therefore be nice if the test failed if we did change to emitting null so that we could update the test at

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

DHowett-MSFT added a comment. It seems more fragile to check the contents of the protocol rather than the invariant we care most about (its size). I'm willing to do that, and am updating the patch accordingly, but I don't want to commit to a more fragile test than is necessary. Illustratively,

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

theraven added a comment. Isn't it better to test for the correct structure existing in the IR? Repository: rC Clang https://reviews.llvm.org/D45305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

rjmccall accepted this revision. rjmccall added a comment. Hmm. Alright, I guess. Repository: rC Clang https://reviews.llvm.org/D45305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

DHowett-MSFT updated this revision to Diff 141450. DHowett-MSFT added a comment. Added a test per @rjmccall's suggestion. I chose to test this by emitting assembly because the llvm ir is significantly harder to pick apart for the true _size_ of the protocol. Repository: rC Clang

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

rjmccall added a comment. The change LGTM, but please add a test. Repository: rC Clang https://reviews.llvm.org/D45305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

theraven added a comment. Looks correct to me. Protocols are only referenced by pointer, so it doesn't matter for old versions of the runtime if these fields are present. Repository: rC Clang https://reviews.llvm.org/D45305 ___ cfe-commits

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

DHowett-MSFT created this revision. DHowett-MSFT added reviewers: rjmccall, theraven. DHowett-MSFT added a project: clang. Herald added a subscriber: cfe-commits. Protocols that were being referenced but could not be fully realized were being emitted without `properties`/`optional_properties`.