[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

David Chisnall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 16 01:26:43 PDT 2018


theraven added inline comments.


================
Comment at: lib/AST/MicrosoftMangle.cpp:1886
   case BuiltinType::ObjCId:
-    mangleArtificalTagType(TTK_Struct, "objc_object");
+    mangleArtificalTagType(TTK_Struct, ".objc_object");
     break;
----------------
smeenai wrote:
> theraven wrote:
> > compnerd wrote:
> > > DHowett-MSFT wrote:
> > > > I'm interested in @smeenai's take on this, as well.
> > > Hmm, doesn't this break ObjC/ObjC++ interoperability?  I don't think that we should be changing the decoration here for the MS ABI case.
> > No, this fixes ObjC++ interop.  Throwing an exception from a `@throw` and catching it with `catch` now works and we avoid the problem that a C++ compilation unit declares a `struct` that happens to have the same name as an Objective-C class (which is very common for wrapper code) may catch things of the wrong type.
> I've seen a lot of code which does something like this in a header
> 
> ```lang=cpp
> #ifdef __OBJC__
> @class I;
> #else
> struct I;
> #endif
> 
> void f(I *);
> ```
> 
> You want `f` to be mangled consistently across C++ and Objective-C++, otherwise you'll get link errors if `f` is e.g. defined in a C++ TU and referenced in an Objective-C++ TU. This was the case before your change and isn't the case after your change, which is what @compnerd was pointing out. They are mangled consistently in the Itanium ABI, and we want them to be mangled consistently in the MS ABI as well.
> 
> Not wanting the catch mix-up is completely reasonable, of course, but it can be done in a more targeted way. I'll put up a patch that undoes this part of the change while still preventing incorrect catching.
With a non-fragile ABI, there is not guarantee that `struct I` and `@class I` have the same layout.  This is why `@defs` has gone away.  Any code that does this and treats `struct I` as anything other than an opaque type is broken.  No other platform supports throwing an Objective-C object and catching it as a struct, and this is an intentional design decision.  I would be strongly opposed to a change that intentionally supported this, because it is breaking correct code in order to make incorrect code do something that may or may not work.


Repository:
  rC Clang

https://reviews.llvm.org/D50144





More information about the cfe-commits mailing list