rjmccall accepted this revision. rjmccall added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang/lib/CodeGen/CGObjCMac.cpp:3812 llvm::GlobalVariable *GV; - if (ForClass) - GV = ---------------- aaron.ballman wrote: > I agree this is dead code (see line 3777-3778 above), but @rjmccall is it a > bug that we bail out with a null value (e.g, is the correct fix to remove the > above lines)? No, I don't think this is a bug, and that would not be the right fix anyway. `ForClass` means we're building the metaclass, and subclasses do not introduce new ivars in the metaclass. The comment is saying that GCC declares ivars for the class structure when emitting metaclasses for root classes, but if we haven't been doing that for 15 years, I don't see a reason to start. Certainly the runtime doesn't require it in order to work. So the patch as written looks good. For what it's worth, this code is all but dead for more basic reasons. `CGObjCMac` is the legacy ObjC ABI, which is no longer used on any platform that's even vaguely supported by Apple. The last platform we shipped using this ABI was i386 macOS; we haven't sold Macs that only supported i386 since 2006, and we dropped the i386 userspace entirely in macOS Catalina (2019). So the only reason for this code to exist is to allow people to either compile programs for very old hardware or intentionally use a 32-bit ABI on newer hardware running an old OS. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152689/new/ https://reviews.llvm.org/D152689 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
