Add support for Objective-C class/static methods by jacob-carlborg · Pull Request #7762 · dlang/dmd

jacob-carlborg · 2018-01-22T22:16:22Z

A few missing things:

The header files need to be updated
Could use some more documentation
Need to add changelog
A PR to update the documentation in dlang.org

Documentation of the Objective-C ABI has been started, available in a separate branch: https://github.com/jacob-carlborg/dmd/blob/objc_abi_docs/docs/objective-c_abi.md.

The dlang.org part dlang/dlang.org#2246.

dlang-bot · 2018-01-22T22:16:23Z

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.

stefan-koch-sociomantic · 2018-01-25T18:06:59Z

There is not a word about what this actually does.

jacob-carlborg · 2018-01-25T20:32:29Z

There is not a word about what this actually does.

Yeah, that's why the unchecked list item, "Could use some more documentation", in the description.

jacob-carlborg · 2018-01-25T21:37:18Z

@stefan-koch-sociomantic On a high level the Objective-C ABI looks something like this:

extern (Objective-C) interface NSObject
{
    NSObject init() @selector("init");
}
NSObject o;
o.init();

The above method call is translated to a regular C function call in the form of: objc_msgSend(o, "init"). o would be the this pointer and "init" is what was specified in the @selector UDA in the method declaration. That is for a regular instance method. For a class method it works the same way, it's just a different this pointer. In Objective-C, a class is an object and it's an instance of a metaclass. An instance of ClassDeclaration is used as the Objective-C metaclass.

For a more detailed explanation see the commit message of the first phase of the Objective-C integration 867d547.

There's also this Wikipedia page about Objective-C metaclasses [1].

[1] https://en.wikipedia.org/wiki/Metaclass#In_Objective-C

ibuclaw · 2018-02-02T08:29:53Z

docs/objective-c_abi.md

+
+There are several Objective-C ABIs available:
+
+* Apple legacy ABI (also know as fragile ABI) - used on macOS 32bit and the old


Just so I have the link correct in my head, this is also known as the Nextstep abi?

Yes. I've moved the work on the ABI documentation to a separate branch until it's ready [1]. The description in the separate branch is a bit more clear and thorough.

[1] https://github.com/jacob-carlborg/dmd/blob/objc_abi_docs/docs/objective-c_abi.md

OK.

I've just checked, and gcc objc seems to support all of these ABIs. I should see if I can re-use the existing code sometime in gdc.

Before Apple used Clang, they used GCC with this ABI that is implemented here. I don't think anything has changed since then. I'm not sure if that was only supported in their own version of GCC or in the standard version as well.

BTW, some code review would be nice 😉.

JinShil · 2018-02-18T00:35:16Z

I don't see any issue with the code in this PR, but I don't know anything about Objective-C, or what D's goals are in supporting it. Isn't Objective-C reaching end of life with the emergence of Swift?

jacob-carlborg · 2018-02-18T08:20:42Z

I don't see any issue with the code in this PR, but I don't know anything about Objective-C, or what D's goals are in supporting it.

D's goals are to support as much of Objective-C as possible or what's necessary to implement meaningful Cocoa applications [1].

Isn't Objective-C reaching end of life with the emergence of Swift?

All of the Apple frameworks are still implemented in Objective-C. Apple is not, most likely, going to replace them in the near future. Swift can interface with Objective-C as well, which is a crucial part since all the frameworks are written in Objective-C. That means that D can interface with Swift code as well, assuming both use the Objective-C interface. Apple might implement new frameworks using Swift and possible rewrite existing frameworks as well, but they would need to support an Objective-C interface for a significant future.

Why was the WIP label added? I think it's possible to review even if some documentation is missing.

[1] https://github.com/dlang/DIPs/blob/master/DIPs/archive/DIP43.md

JinShil · 2018-02-18T08:33:04Z

Why was the WIP label added? I think it's possible to review even if some documentation is missing.

Documentation helps the review, and if, from the author's perspective, a PR is not complete enough to be merged, it is indeed a WIP.

jacob-carlborg · 2018-02-18T08:47:36Z

Documentation helps the review, and if, from the author's perspective, a PR is not complete enough to be merged, it is indeed a WIP.

Well, it depends. I think the code is ready as is. Documentation is missing yes, but keep in mind that most code in DMD was reviewed and merged without documentation. I mean, I don't see any documentation describing any of the C++ ABIs.

JinShil · 2018-02-18T09:00:38Z

src/dmd/objc.d

+            else
+            {
+                error("base " ~ errorType ~ " for an Objective-C " ~
+                      errorType ~ " must be extern (Objective-C)");


tick marks around extern (Objective-C), I believe.

Yes, good catch.

WalterBright · 2018-02-19T22:07:54Z

src/dmd/objc_glue.d

+
+private:
+
+char[] format(size_t bufLength, Args...)(return ref char[bufLength] buffer,


Ddoc comment

Yes, that's why the "Could use some more documentation" checkbox in the PR description is not checked 😃.

WalterBright · 2018-02-19T22:08:14Z

src/dmd/objc_glue.d

+        return symbol;
+    }
+
+    Symbol* getClassReference(const ClassDeclaration classDeclaration)


Ddoc comment

WalterBright · 2018-02-19T22:08:23Z

src/dmd/objc_glue.d

        return moduleInfo;
    }

+    Symbol* getClassName(const ClassDeclaration classDeclaration)


Ddoc comment

WalterBright · 2018-02-19T22:08:47Z

src/dmd/objc_glue.d

        return false;
    }

+    Symbol* symbolName(const(char)[] name, int sclass, type* t)


Ddoc comment for these three

WalterBright · 2018-02-19T22:09:56Z

src/dmd/objc.d

    }
 }
+
+private void setMetaclass(alias newMetaclass, T)(T classDeclaration, Scope* sc)


Ddoc comment

WalterBright · 2018-02-19T22:11:02Z

src/dmd/objc.d


+
+/**
+ * This struct contains all data for a class declaration that is needed for the


remove This struct contains. It's redundant.

src/dmd/objc.d

jacob-carlborg · 2018-02-20T16:15:51Z

Please see #7923 as well.

jacob-carlborg · 2018-02-23T13:36:31Z

Added documentation.

jacob-carlborg · 2018-02-23T15:52:22Z

Created a PR for the dlang.org part now as well: dlang/dlang.org#2246.

Everything on my todo list has been completed. This is ready to go.

jacob-carlborg · 2018-03-03T19:43:17Z

@JinShil more than 72 hours have passed.

jacob-carlborg · 2018-03-04T08:21:11Z

Thanks.

jacob-carlborg force-pushed the objc_class_methods branch 4 times, most recently from 60c78e8 to 2019571 Compare January 23, 2018 15:40

jacob-carlborg requested review from dnadlinger and ibuclaw as code owners January 23, 2018 15:40

jacob-carlborg force-pushed the objc_class_methods branch 3 times, most recently from 741d089 to 4eadda2 Compare January 25, 2018 16:04

jacob-carlborg force-pushed the objc_class_methods branch 2 times, most recently from 9217b2c to ff304fa Compare January 28, 2018 22:19

ibuclaw reviewed Feb 2, 2018

View reviewed changes

jacob-carlborg mentioned this pull request Feb 6, 2018

fix Issue 11847 - sub-pkg not available as qualified name #7218

Closed

JinShil added the Review:WIP Work In Progress - not ready for review or pulling label Feb 18, 2018

JinShil reviewed Feb 18, 2018

View reviewed changes

jacob-carlborg force-pushed the objc_class_methods branch from ff304fa to 860a7f4 Compare February 18, 2018 13:22

WalterBright reviewed Feb 19, 2018

View reviewed changes

src/dmd/objc.d Show resolved Hide resolved

jacob-carlborg force-pushed the objc_class_methods branch 2 times, most recently from 87eafec to 201059e Compare February 23, 2018 13:36

Add support for Objective-C class/static methods

bf824c6

jacob-carlborg force-pushed the objc_class_methods branch from 201059e to bf824c6 Compare February 23, 2018 13:47

jacob-carlborg mentioned this pull request Feb 23, 2018

Update the Objective-C documentation with support for class methods dlang/dlang.org#2246

Merged

JinShil removed the Review:WIP Work In Progress - not ready for review or pulling label Feb 28, 2018

JinShil approved these changes Feb 28, 2018

View reviewed changes

JinShil added the Merge:72h no objection -> merge The PR will be merged if there are no objections raised. label Feb 28, 2018

JinShil added the Merge:auto-merge label Mar 3, 2018

dlang-bot merged commit acacaad into dlang:master Mar 3, 2018

jacob-carlborg deleted the objc_class_methods branch March 4, 2018 08:21

jacob-carlborg mentioned this pull request Mar 5, 2018

Start documenting the Objective-C ABI #7923

Merged

kinke mentioned this pull request Apr 27, 2018

Add support for Objective-C class/static methods (2.080 feature) ldc-developers/ldc#2670

Closed


		There are several Objective-C ABIs available:

		* Apple legacy ABI (also know as fragile ABI) - used on macOS 32bit and the old


		private:

		char[] format(size_t bufLength, Args...)(return ref char[bufLength] buffer,



		/**
		* This struct contains all data for a class declaration that is needed for the

Uh oh!

Conversation

jacob-carlborg commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dlang-bot commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bugzilla references

Uh oh!

stefan-koch-sociomantic commented Jan 25, 2018

Uh oh!

jacob-carlborg commented Jan 25, 2018

Uh oh!

jacob-carlborg commented Jan 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JinShil commented Feb 18, 2018

Uh oh!

jacob-carlborg commented Feb 18, 2018

Uh oh!

JinShil commented Feb 18, 2018

Uh oh!

jacob-carlborg commented Feb 18, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jacob-carlborg commented Feb 20, 2018

Uh oh!

jacob-carlborg commented Feb 23, 2018

Uh oh!

jacob-carlborg commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jacob-carlborg commented Mar 3, 2018

Uh oh!

jacob-carlborg commented Mar 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jacob-carlborg commented Jan 22, 2018 •

edited

Loading

dlang-bot commented Jan 22, 2018 •

edited

Loading

jacob-carlborg commented Jan 25, 2018 •

edited

Loading

jacob-carlborg commented Feb 23, 2018 •

edited

Loading