Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update: forward declarations in c and objc #4477

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Conversation

danil-pavlov
Copy link
Contributor

No description provided.

docs/topics/native/native-c-interop.md Outdated Show resolved Hide resolved
docs/topics/native/native-c-interop.md Outdated Show resolved Hide resolved
docs/topics/native/native-c-interop.md Outdated Show resolved Hide resolved
docs/topics/native/native-c-interop.md Outdated Show resolved Hide resolved
docs/topics/native/native-objc-interop.md Outdated Show resolved Hide resolved
docs/topics/native/native-objc-interop.md Outdated Show resolved Hide resolved
docs/topics/native/native-objc-interop.md Outdated Show resolved Hide resolved
docs/topics/native/native-objc-interop.md Outdated Show resolved Hide resolved
docs/topics/native/native-objc-interop.md Outdated Show resolved Hide resolved
Copy link
Member

@koshachy koshachy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!
Please address my comments before merging.

@@ -914,7 +914,7 @@ Here's the planned deprecation cycle:
**What's changed?**

The JetBrains team has revamped the approach to forward declarations in Kotlin to make their behavior more predictable
and prepare this functionality for the upcoming Kotlin 2.0 release. From now on:
and prepare this functionality for the Kotlin 2.0.0 release. From now on:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this line about Kotlin 2.0.0? In my opinion, the following line is more than enough:

"The JetBrains team has revamped the approach to forward declarations in Kotlin to make their behavior more predictable:"

> Otherwise, you'll get an error.
>
{style="note"}
* Consider two objcinterop libraries, one that uses `objcnames.protocols.ForwardDeclaredProtocolProtocol` and the other
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Consider two objcinterop libraries, one that uses `objcnames.protocols.ForwardDeclaredProtocolProtocol` and the other
* Consider two objcinterop libraries: one that uses `objcnames.protocols.ForwardDeclaredProtocolProtocol` and another

}
```

> The casting to `objcnames.protocols.ForwardDeclaredProtocolProtocol` is only allowed from the corresponding real class.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is better to get rid of the passive voice here:

You can only cast to objcnames.protocols.ForwardDeclaredProtocolProtocol from the corresponding real class. Otherwise, you'll get an error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you'd like to keep the original sentence, the first "The" article is not needed.

`import cnames.structs.cstructName`.

Consider two cinterop libraries, one that has a forward declaration of a struct and the other
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the suggestion above.

```

To transfer objects between the two libraries, make the `as` cast in you Kotlin code:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To transfer objects between the two libraries, make the `as` cast in you Kotlin code:
To transfer objects between the two libraries, use the `as` cast in you Kotlin code:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds more natural to me.

@@ -228,7 +228,11 @@ A Swift/Objective-C initializer is imported to Kotlin as constructors or as fact
The latter happens with initializers declared in the Objective-C category or as a Swift extension,
because Kotlin has no concept of extension constructors.

Kotlin constructors are imported as initializers to Swift/Objective-C.
> Remember to annotate Swift initializers with `@objc` before importing them to Kotlin.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a recommendation or a mandatory action?
If the latter, we can omit the "Remember" word:

Annotate Swfit initializers ...

or

"Before importing Swift initializers to Kotlin, add the @objc annotation to them"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, here is just a reminder of something stated above. Some people skip it, so we decided to add a note in this section too

to import a `objcprotocolName` forward declaration declared in an Objective-C library with a `library.package`,
use a special forward declaration package: `import objcnames.protocols.objcprotocolName`.

Consider two objcinterop libraries, one that uses `objcnames.protocols.ForwardDeclaredProtocolProtocol`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the suggestion above.

}
```

> The casting to `objcnames.protocols.ForwardDeclaredProtocolProtocol` is only allowed from the corresponding real class.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the suggestion above.

@danil-pavlov danil-pavlov merged commit ec7a083 into master Oct 22, 2024
4 checks passed
@danil-pavlov danil-pavlov deleted the c-struct-handlling branch October 22, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants