Add support for Objective-C classes #9179

jacob-carlborg · 2019-01-01T08:50:16Z

This PR contains a bit more code then I would like but it's difficult to reduce it anymore.

The PR consists of four commits and can be individually reviewed but doesn't make sense to split up in multiple PRs.

dlang-bot · 2019-01-01T08:50:17Z

Thanks for your pull request and interest in making D better, @jacob-carlborg! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#9179"

Geod24 · 2019-01-01T11:50:38Z

changelog/objc_class.dd

+$(UL
+    $(LI `static` and `final` methods are virtual. Although `final` methods are
+         virtual it's not possible to override a `final` method in a subclass)
+    $(LI `static` methods are overridable in subclasses)


That sounds quite fishy. An extern declaration is used for compatibility, but we always stayed away from importing other language's semantic into D, as it's a hornets nest. It's fine to have final method as virtual if ObjC has no way to represent it, but allowing override static is downright confusing.

I assume this comes from the necessity to have it in the vtable, even if its static ?

Class/static methods in Objective-C are instance methods on the metaclass. In Objective-C classes are object themselves.

I assume this comes from the necessity to have it in the vtable, even if its static ?

There is no vtable for Objective-C classes. All method calls are lowered to a function in the Objective-C runtime.

Without this, it is basically impossible to actually use existing Objective C classes. We sometimes need to be practical even when it sounds weird...

@jacob-carlborg : Is there a way to expose this without changing the language ?

Expose what? That static methods are overridable?

Geod24 · 2019-01-03T00:55:04Z

Using the HEAD of this PR (`DMD64 D Compiler v2.084.0-rc.1-161-g4b99aba26) and slightly modifying the example code:

void main()
{
    auto foo = Foo.alloc.init;
    scope (exit) foo.release();

    assert(foo.bar(3) == 3);

    pragma(msg, &__traits(getOverloads, Foo, "alloc"));
}

extern (Objective-C):
class NSObject
{
     static NSObject alloc() @selector("alloc");
     NSObject init() @selector("init");
     void release() @selector("release");
 }

class Foo : NSObject
{
    override static Foo alloc() @selector("alloc");
    override Foo init() @selector("init");

    int bar(int a) @selector("bar:")
    {
        return a;
    }
}

~/projects/dmd (git)-[remotes/jacob/objc-class] % ./generated/osx/release/64/dmd -run test.d                 :(
---
ERROR: This is a compiler bug.
Please report it via https://issues.dlang.org/enter_bug.cgi
with, preferably, a reduced, reproducible example and the information below.
DustMite (https://github.com/CyberShadow/DustMite/wiki) can help with the reduction.
---
DMD v2.084.0-rc.1-161-g4b99aba26
predefs   DigitalMars Posix OSX CppRuntime_Clang darwin LittleEndian D_Version2 all D_SIMD D_InlineAsm_X86_64 X86_64 D_LP64 D_PIC assert D_ModuleInfo D_Exceptions D_TypeInfo D_HardFloat D_ObjectiveC
binary    ./generated/osx/release/64/dmd
version   v2.084.0-rc.1-161-g4b99aba26
config    ./generated/osx/release/64/dmd.conf
DFLAGS    -I./generated/osx/release/64/../../../../../druntime/import -I./generated/osx/release/64/../../../../../phobos -L-L./generated/osx/release/64/../../../../../phobos/generated/osx/release/64 -fPIC
---
core.exception.AssertError@dmd/hdrgen.d(2300): Assertion failure
----------------
??:? _d_assertp [0xb92f689]
dmd/access.d:623 _ZN18PrettyPrintVisitor11expToBufferEP10Expression4PREC [0xb795831]
dmd/access.d:623 _ZN18PrettyPrintVisitor5visitEP9DotVarExp [0xb79778d]
dmd/access.d:623 _ZN9DotVarExp6acceptEP7Visitor [0xb75d395]
[...]

WalterBright · 2019-01-03T00:58:48Z

doc/objectivec-abi.md

The official documentation is in Ddoc .dd files. Trying to mix and match .md and .dd files will cause a lot of bugs and effort for others trying to build the dlang website.

src/dmd/declaration.d

wilzbach · 2019-01-03T01:06:06Z

doc/objectivec-abi.md

The official documentation is in Ddoc .dd files. Trying to mix and match .md and .dd files will cause a lot of bugs and effort for others trying to build the dlang website.

These files aren't part of the official user documentation, but for developers and in markdown as that's what GitHub understands when you browse those files on the web (which only DMD devs or very interested people would do).

src/dmd/func.d

src/dmd/identifier.d

src/dmd/mars.d

src/dmd/semantic2.d

src/dmd/objc.d

thewilsonator · 2019-01-06T10:53:47Z

Thanks.

jacob-carlborg · 2019-01-06T16:37:01Z

@Geod24 that error in #9179 (comment) is not something new in this PR. It exists in current DMD. Well, it segfaults rather than asserts.

jacob-carlborg · 2019-01-06T20:00:16Z

I've fixed the #9179 (comment) comment now. I moved all the Objective-C specific semantic functions for function declaration to the first semantic phase, where the original were located.

I'll leave the issue that @Geod24 mentioned because it was already present before this PR. It doesn't look like it's possible to take the address of a method in Objective-C. It's possible to replace methods at runtime in Objective-C, but the Objective-C runtime provides functions to get access to the actual implementation. Because of this I think we should make it illegal to take the address of a function with Objective-C linkage in D.

jacob-carlborg · 2019-01-06T20:04:12Z

@thewilsonator This is good to go, unless I should change the implementation of anonymous variable declarations, see: #9179 (comment).

thewilsonator · 2019-01-07T08:16:55Z

OSX64 auto-tester failed with

 ... fail_compilation/objc_class3.d  -fPIC ()
Test fail_compilation/objc_class3.d failed.  The logged output:
/Users/braddr/sandbox/at-client/pull-3472605-Darwin_64_64/dmd/generated/osx/release/64/dmd -conf= -m64 -Ifail_compilation -verrors=0  -fPIC  -odgenerated/fail_compilation -ofgenerated/fail_compilation/objc_class3_0.o  -c fail_compilation/objc_class3.d 


==============================
Test fail_compilation/objc_class3.d failed: expected rc == 1, but exited with rc == 0

make[2]: *** [generated/fail_compilation/objc_class3.d.out] Error 1

Not particularly helpful message.

jacob-carlborg · 2019-01-07T08:20:04Z

OSX64 auto-tester failed with

I’ll have a look.

jacob-carlborg · 2019-01-07T11:14:18Z

Not particularly helpful message.

It's a test that should fail to compile to succeeds. Hence the unhelpful message.

Representing an Objective-C class as a D interface has been deprecated. Classes have now been properly implemented and the `class` keyword should be used instead. In the future, `extern(Objective-C)` interfaces will be used to represent Objective-C protocols.

jacob-carlborg · 2019-01-07T21:39:48Z

Looks like the tests are passing now.

jacob-carlborg · 2019-01-08T10:26:04Z

@thewilsonator everything is green.

thewilsonator · 2019-01-08T11:34:47Z

Auto-merge toggled on

wilzbach · 2019-01-08T12:09:20Z

Auto-merge toggled on

Why don't you use "auto-merge"?
It works very reliably and actually doesn't merge it automatically when there's a CI failure as these should always be inspected manually (or be avoided in the first place).

jacob-carlborg · 2019-01-08T12:10:24Z

Why don't you use "auto-merge"?

Seems he toggled it in the auto-tester UI and not here on GitHub.

thewilsonator · 2019-01-08T12:13:53Z

Bingo.

I need to upgrade my system because GH has dropped support for it and broke almost everything in the process.

wilzbach · 2019-01-08T12:32:44Z

I need to upgrade my system because GH has dropped support for it and broke almost everything in the process.

So you're saying that your Chrome/Firefox doesn't allow you to add this anymore:

FWIW it is still just a one-liner with their API and curl:

curl -u "wilzbach:$GITHUB_OAUTH_TOKEN" -X POST -d '{"labels": ["auto-merge"]}' https://api.github.com/repos/dlang/dmd/issues/9179/labels

jacob-carlborg · 2019-01-08T13:28:26Z

I have an old iPad stuck at iOS 9. I can't add inline comments or set labels on that because the browser is too old.

jacob-carlborg requested review from dnadlinger and ibuclaw as code owners January 1, 2019 08:50

jacob-carlborg added the Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org label Jan 1, 2019

jacob-carlborg force-pushed the objc-class branch 3 times, most recently from 5f0e7c2 to b1be7a6 Compare January 1, 2019 09:23

jacob-carlborg requested review from Geod24, MartinNowak, WalterBright and rainers as code owners January 1, 2019 09:23

Geod24 reviewed Jan 1, 2019

View reviewed changes

jacob-carlborg force-pushed the objc-class branch 3 times, most recently from 87b2ad8 to 4b99aba Compare January 1, 2019 15:19

jacob-carlborg mentioned this pull request Jan 2, 2019

Add documentation for Objective-C classes dlang/dlang.org#2540

Merged

jacob-carlborg removed the Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org label Jan 2, 2019