Feature Proposal to improve Mixed Frameworks support


(Gonzalo Larralde) #1

I wanted to bring a few points into attention to discuss potential
improvements on Mixed Frameworks support. The current status for Mixed
Frameworks present some challenges, one specifically being how the Swift
compiler generates the ObjC Interface Header.

At this moment, there's a detection procedure based on the presence of an
entry point definition (either using the main file argument, or
@UIApplicationMain). The ObjC Interface Header writer
(swift/lib/PrintAsObjC/PrintAsObjC.cpp) is using this detection to
determine if the minimum accessibility level to be exported is either
`internal` (for app targets) or `public` (for framework targets). This can
be observed here:
https://github.com/apple/swift/blob/master/lib/FrontendTool/FrontendTool.cpp#L652-L657

The result of this is a difference on how default visibility is exposed to
ObjC code in the same compilation scope.

Having this initial piece of code:

public class Test: NSObject {
public foo() {}
func bar() {}
}

results on the following Interface Header for Main App targets

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
- (void)bar;
@end

and the following for Frameworks

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
@end

This is clearly correct for the publicly visible interface of the
framework, but not quite the expected result for the ObjC code compiled in
the same target. In that scenario it would make more sense to me that all
the `internal` methods are visible.

A potential solution for this problem is to generate two Interface Headers,
one that exports all the `public` and `open` entities and members, and an
additional header exposing internal entities and declaring categories to
public entities and exposing internal members.

Something like:

<target-name>-swift-internal.h

@interface Test (InternalMembers)
- (void)bar;
@end

After that it'll be Xcode's responsability to create both files, and mark
them as Public and Project in the Headers Build Phase.

An initial implementation that I think would make sense in case this
solution is accepted could be modifying the signature of
`swift::printAsObjC` to require the list of access levels to export as a
bitmask instead of just getting the minimum. That will enable the exporter
to create the following set of files:

* Internal, Public, Open > <target-name>-swift.h for app targets
* Public, Open > <target-name>-swift.h for framework targets
* Internal > <target-name>-swift-internal.h for the internal entities and
members on framework targets.

To make this happen a new argument needs to be passed to the compiler call.
When it's not passed the default behaviour would remain the same, but when
it is the behaviour needs to be explicitly defined. One option is to just
get the explicit list of access levels to export (something like
`-export=internal,public,open`) or export levels to be defined (0 for app
targets, 1 for public framework targets and 2 for the internal header
framework targets for example)

I hope this feature proposal is complete enough to properly define the
scope and discuss the viability of a possible solution. Otherwise please
let me know.

Thanks.

···

--
Slds,

Gonzalo.


(Jordan Rose) #2

Thanks for following through on this, Gonzalo! (The two of us talked about this briefly at WWDC.) My idea was a little less clever than yours: just always include the internal members in the main header unless you provide the name of a second header to put them in. (I called this flag -emit-objc-internal-header-path.) That's not quite as flexible, but does remove all the guesswork, at least for generated headers.

I filed a bug to track this work: SR-5221 <https://bugs.swift.org/browse/SR-5221>.

Jordan

···

On Jun 29, 2017, at 06:35, Gonzalo Larralde via swift-dev <swift-dev@swift.org> wrote:

I wanted to bring a few points into attention to discuss potential improvements on Mixed Frameworks support. The current status for Mixed Frameworks present some challenges, one specifically being how the Swift compiler generates the ObjC Interface Header.

At this moment, there's a detection procedure based on the presence of an entry point definition (either using the main file argument, or @UIApplicationMain). The ObjC Interface Header writer (swift/lib/PrintAsObjC/PrintAsObjC.cpp) is using this detection to determine if the minimum accessibility level to be exported is either `internal` (for app targets) or `public` (for framework targets). This can be observed here: https://github.com/apple/swift/blob/master/lib/FrontendTool/FrontendTool.cpp#L652-L657

The result of this is a difference on how default visibility is exposed to ObjC code in the same compilation scope.

Having this initial piece of code:

public class Test: NSObject {
	public foo() {}
	func bar() {}
}

results on the following Interface Header for Main App targets

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
- (void)bar;
@end

and the following for Frameworks

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
@end

This is clearly correct for the publicly visible interface of the framework, but not quite the expected result for the ObjC code compiled in the same target. In that scenario it would make more sense to me that all the `internal` methods are visible.

A potential solution for this problem is to generate two Interface Headers, one that exports all the `public` and `open` entities and members, and an additional header exposing internal entities and declaring categories to public entities and exposing internal members.

Something like:

<target-name>-swift-internal.h

@interface Test (InternalMembers)
- (void)bar;
@end

After that it'll be Xcode's responsability to create both files, and mark them as Public and Project in the Headers Build Phase.

An initial implementation that I think would make sense in case this solution is accepted could be modifying the signature of `swift::printAsObjC` to require the list of access levels to export as a bitmask instead of just getting the minimum. That will enable the exporter to create the following set of files:

* Internal, Public, Open > <target-name>-swift.h for app targets
* Public, Open > <target-name>-swift.h for framework targets
* Internal > <target-name>-swift-internal.h for the internal entities and members on framework targets.

To make this happen a new argument needs to be passed to the compiler call. When it's not passed the default behaviour would remain the same, but when it is the behaviour needs to be explicitly defined. One option is to just get the explicit list of access levels to export (something like `-export=internal,public,open`) or export levels to be defined (0 for app targets, 1 for public framework targets and 2 for the internal header framework targets for example)

I hope this feature proposal is complete enough to properly define the scope and discuss the viability of a possible solution. Otherwise please let me know.

Thanks.

--
Slds,

Gonzalo.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Daniel Dunbar) #3

Thanks for following through on this, Gonzalo! (The two of us talked about this briefly at WWDC.) My idea was a little less clever than yours: just always include the internal members in the main header unless you provide the name of a second header to put them in. (I called this flag -emit-objc-internal-header-path.) That's not quite as flexible, but does remove all the guesswork, at least for generated headers.

Doesn't that mean downstream Obj-C clients get bogus code completion results?

I filed a bug to track this work: SR-5221 <https://bugs.swift.org/browse/SR-5221>.

Cool, this sounds like a nice feature to me, I have heard several complaints about needing to mark things public unnecessarily.

- Daniel

···

On Jun 29, 2017, at 10:43 AM, Jordan Rose via swift-dev <swift-dev@swift.org> wrote:

Jordan

On Jun 29, 2017, at 06:35, Gonzalo Larralde via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

I wanted to bring a few points into attention to discuss potential improvements on Mixed Frameworks support. The current status for Mixed Frameworks present some challenges, one specifically being how the Swift compiler generates the ObjC Interface Header.

At this moment, there's a detection procedure based on the presence of an entry point definition (either using the main file argument, or @UIApplicationMain). The ObjC Interface Header writer (swift/lib/PrintAsObjC/PrintAsObjC.cpp) is using this detection to determine if the minimum accessibility level to be exported is either `internal` (for app targets) or `public` (for framework targets). This can be observed here: https://github.com/apple/swift/blob/master/lib/FrontendTool/FrontendTool.cpp#L652-L657

The result of this is a difference on how default visibility is exposed to ObjC code in the same compilation scope.

Having this initial piece of code:

public class Test: NSObject {
	public foo() {}
	func bar() {}
}

results on the following Interface Header for Main App targets

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
- (void)bar;
@end

and the following for Frameworks

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
@end

This is clearly correct for the publicly visible interface of the framework, but not quite the expected result for the ObjC code compiled in the same target. In that scenario it would make more sense to me that all the `internal` methods are visible.

A potential solution for this problem is to generate two Interface Headers, one that exports all the `public` and `open` entities and members, and an additional header exposing internal entities and declaring categories to public entities and exposing internal members.

Something like:

<target-name>-swift-internal.h

@interface Test (InternalMembers)
- (void)bar;
@end

After that it'll be Xcode's responsability to create both files, and mark them as Public and Project in the Headers Build Phase.

An initial implementation that I think would make sense in case this solution is accepted could be modifying the signature of `swift::printAsObjC` to require the list of access levels to export as a bitmask instead of just getting the minimum. That will enable the exporter to create the following set of files:

* Internal, Public, Open > <target-name>-swift.h for app targets
* Public, Open > <target-name>-swift.h for framework targets
* Internal > <target-name>-swift-internal.h for the internal entities and members on framework targets.

To make this happen a new argument needs to be passed to the compiler call. When it's not passed the default behaviour would remain the same, but when it is the behaviour needs to be explicitly defined. One option is to just get the explicit list of access levels to export (something like `-export=internal,public,open`) or export levels to be defined (0 for app targets, 1 for public framework targets and 2 for the internal header framework targets for example)

I hope this feature proposal is complete enough to properly define the scope and discuss the viability of a possible solution. Otherwise please let me know.

Thanks.

--
Slds,

Gonzalo.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org <mailto:swift-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-dev

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Gonzalo Larralde) #4

Ah, great, that's even better. My solution is overkill compared to this and
I think you're covering all the interesting cases anyway. I would love to
work on the implementation if it's not assigned to anyone internally yet.
(I've seen you mentioned there's a radar already?)

Cheers.

···

--
Slds,

Gonzalo.

On Thu, Jun 29, 2017 at 2:42 PM, Jordan Rose <jordan_rose@apple.com> wrote:

Thanks for following through on this, Gonzalo! (The two of us talked about
this briefly at WWDC.) My idea was a little less clever than yours: just
always include the internal members in the main header *unless* you
provide the name of a second header to put them in. (I called this flag
-emit-objc-internal-header-path.) That's not quite as flexible, but does
remove all the guesswork, at least for generated headers.

I filed a bug to track this work: SR-5221
<https://bugs.swift.org/browse/SR-5221>.

Jordan

On Jun 29, 2017, at 06:35, Gonzalo Larralde via swift-dev < > swift-dev@swift.org> wrote:

I wanted to bring a few points into attention to discuss potential
improvements on Mixed Frameworks support. The current status for Mixed
Frameworks present some challenges, one specifically being how the Swift
compiler generates the ObjC Interface Header.

At this moment, there's a detection procedure based on the presence of an
entry point definition (either using the main file argument, or
@UIApplicationMain). The ObjC Interface Header writer
(swift/lib/PrintAsObjC/PrintAsObjC.cpp) is using this detection to
determine if the minimum accessibility level to be exported is either
`internal` (for app targets) or `public` (for framework targets). This can
be observed here: https://github.com/apple/swift/blob/master/lib/
FrontendTool/FrontendTool.cpp#L652-L657

The result of this is a difference on how default visibility is exposed to
ObjC code in the same compilation scope.

Having this initial piece of code:

public class Test: NSObject {
public foo() {}
func bar() {}
}

results on the following Interface Header for Main App targets

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
- (void)bar;
@end

and the following for Frameworks

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
@end

This is clearly correct for the publicly visible interface of the
framework, but not quite the expected result for the ObjC code compiled in
the same target. In that scenario it would make more sense to me that all
the `internal` methods are visible.

A potential solution for this problem is to generate two Interface
Headers, one that exports all the `public` and `open` entities and members,
and an additional header exposing internal entities and declaring
categories to public entities and exposing internal members.

Something like:

<target-name>-swift-internal.h

@interface Test (InternalMembers)
- (void)bar;
@end

After that it'll be Xcode's responsability to create both files, and mark
them as Public and Project in the Headers Build Phase.

An initial implementation that I think would make sense in case this
solution is accepted could be modifying the signature of
`swift::printAsObjC` to require the list of access levels to export as a
bitmask instead of just getting the minimum. That will enable the exporter
to create the following set of files:

* Internal, Public, Open > <target-name>-swift.h for app targets
* Public, Open > <target-name>-swift.h for framework targets
* Internal > <target-name>-swift-internal.h for the internal entities and
members on framework targets.

To make this happen a new argument needs to be passed to the compiler
call. When it's not passed the default behaviour would remain the same, but
when it is the behaviour needs to be explicitly defined. One option is to
just get the explicit list of access levels to export (something like
`-export=internal,public,open`) or export levels to be defined (0 for app
targets, 1 for public framework targets and 2 for the internal header
framework targets for example)

I hope this feature proposal is complete enough to properly define the
scope and discuss the viability of a possible solution. Otherwise please
let me know.

Thanks.

--
Slds,

Gonzalo.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Jordan Rose) #5

Thanks for following through on this, Gonzalo! (The two of us talked about this briefly at WWDC.) My idea was a little less clever than yours: just always include the internal members in the main header unless you provide the name of a second header to put them in. (I called this flag -emit-objc-internal-header-path.) That's not quite as flexible, but does remove all the guesswork, at least for generated headers.

Doesn't that mean downstream Obj-C clients get bogus code completion results?

I’m not sure what you mean by this. Frameworks will always generate both headers, apps and unit tests will always generate one header. The only problem is if someone custom-builds a library target and is relying on that not including internal decls…which, admittedly, can happen. But the current logic is a patchwork of guesswork.

···

On Jun 29, 2017, at 16:35, Daniel Dunbar <daniel_dunbar@apple.com> wrote:

On Jun 29, 2017, at 10:43 AM, Jordan Rose via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

I filed a bug to track this work: SR-5221 <https://bugs.swift.org/browse/SR-5221>.

Cool, this sounds like a nice feature to me, I have heard several complaints about needing to mark things public unnecessarily.

- Daniel

Jordan

On Jun 29, 2017, at 06:35, Gonzalo Larralde via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

I wanted to bring a few points into attention to discuss potential improvements on Mixed Frameworks support. The current status for Mixed Frameworks present some challenges, one specifically being how the Swift compiler generates the ObjC Interface Header.

At this moment, there's a detection procedure based on the presence of an entry point definition (either using the main file argument, or @UIApplicationMain). The ObjC Interface Header writer (swift/lib/PrintAsObjC/PrintAsObjC.cpp) is using this detection to determine if the minimum accessibility level to be exported is either `internal` (for app targets) or `public` (for framework targets). This can be observed here: https://github.com/apple/swift/blob/master/lib/FrontendTool/FrontendTool.cpp#L652-L657

The result of this is a difference on how default visibility is exposed to ObjC code in the same compilation scope.

Having this initial piece of code:

public class Test: NSObject {
	public foo() {}
	func bar() {}
}

results on the following Interface Header for Main App targets

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
- (void)bar;
@end

and the following for Frameworks

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
@end

This is clearly correct for the publicly visible interface of the framework, but not quite the expected result for the ObjC code compiled in the same target. In that scenario it would make more sense to me that all the `internal` methods are visible.

A potential solution for this problem is to generate two Interface Headers, one that exports all the `public` and `open` entities and members, and an additional header exposing internal entities and declaring categories to public entities and exposing internal members.

Something like:

<target-name>-swift-internal.h

@interface Test (InternalMembers)
- (void)bar;
@end

After that it'll be Xcode's responsability to create both files, and mark them as Public and Project in the Headers Build Phase.

An initial implementation that I think would make sense in case this solution is accepted could be modifying the signature of `swift::printAsObjC` to require the list of access levels to export as a bitmask instead of just getting the minimum. That will enable the exporter to create the following set of files:

* Internal, Public, Open > <target-name>-swift.h for app targets
* Public, Open > <target-name>-swift.h for framework targets
* Internal > <target-name>-swift-internal.h for the internal entities and members on framework targets.

To make this happen a new argument needs to be passed to the compiler call. When it's not passed the default behaviour would remain the same, but when it is the behaviour needs to be explicitly defined. One option is to just get the explicit list of access levels to export (something like `-export=internal,public,open`) or export levels to be defined (0 for app targets, 1 for public framework targets and 2 for the internal header framework targets for example)

I hope this feature proposal is complete enough to properly define the scope and discuss the viability of a possible solution. Otherwise please let me know.

Thanks.

--
Slds,

Gonzalo.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org <mailto:swift-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-dev

_______________________________________________
swift-dev mailing list
swift-dev@swift.org <mailto:swift-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-dev


(Daniel Dunbar) #6

Thanks for following through on this, Gonzalo! (The two of us talked about this briefly at WWDC.) My idea was a little less clever than yours: just always include the internal members in the main header unless you provide the name of a second header to put them in. (I called this flag -emit-objc-internal-header-path.) That's not quite as flexible, but does remove all the guesswork, at least for generated headers.

Doesn't that mean downstream Obj-C clients get bogus code completion results?

I’m not sure what you mean by this.

I simply misunderstood what you were proposing -- your comments were specifically about using a category to extend; not about adding both members to the existing public header.

- Daniel

···

On Jun 29, 2017, at 4:39 PM, Jordan Rose <jordan_rose@apple.com> wrote:

On Jun 29, 2017, at 16:35, Daniel Dunbar <daniel_dunbar@apple.com <mailto:daniel_dunbar@apple.com>> wrote:

On Jun 29, 2017, at 10:43 AM, Jordan Rose via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

Frameworks will always generate both headers, apps and unit tests will always generate one header. The only problem is if someone custom-builds a library target and is relying on that not including internal decls…which, admittedly, can happen. But the current logic is a patchwork of guesswork.

I filed a bug to track this work: SR-5221 <https://bugs.swift.org/browse/SR-5221>.

Cool, this sounds like a nice feature to me, I have heard several complaints about needing to mark things public unnecessarily.

- Daniel

Jordan

On Jun 29, 2017, at 06:35, Gonzalo Larralde via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

I wanted to bring a few points into attention to discuss potential improvements on Mixed Frameworks support. The current status for Mixed Frameworks present some challenges, one specifically being how the Swift compiler generates the ObjC Interface Header.

At this moment, there's a detection procedure based on the presence of an entry point definition (either using the main file argument, or @UIApplicationMain). The ObjC Interface Header writer (swift/lib/PrintAsObjC/PrintAsObjC.cpp) is using this detection to determine if the minimum accessibility level to be exported is either `internal` (for app targets) or `public` (for framework targets). This can be observed here: https://github.com/apple/swift/blob/master/lib/FrontendTool/FrontendTool.cpp#L652-L657

The result of this is a difference on how default visibility is exposed to ObjC code in the same compilation scope.

Having this initial piece of code:

public class Test: NSObject {
	public foo() {}
	func bar() {}
}

results on the following Interface Header for Main App targets

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
- (void)bar;
@end

and the following for Frameworks

<target-name>-swift.h

@interface Test : NSObject
- (void)foo;
@end

This is clearly correct for the publicly visible interface of the framework, but not quite the expected result for the ObjC code compiled in the same target. In that scenario it would make more sense to me that all the `internal` methods are visible.

A potential solution for this problem is to generate two Interface Headers, one that exports all the `public` and `open` entities and members, and an additional header exposing internal entities and declaring categories to public entities and exposing internal members.

Something like:

<target-name>-swift-internal.h

@interface Test (InternalMembers)
- (void)bar;
@end

After that it'll be Xcode's responsability to create both files, and mark them as Public and Project in the Headers Build Phase.

An initial implementation that I think would make sense in case this solution is accepted could be modifying the signature of `swift::printAsObjC` to require the list of access levels to export as a bitmask instead of just getting the minimum. That will enable the exporter to create the following set of files:

* Internal, Public, Open > <target-name>-swift.h for app targets
* Public, Open > <target-name>-swift.h for framework targets
* Internal > <target-name>-swift-internal.h for the internal entities and members on framework targets.

To make this happen a new argument needs to be passed to the compiler call. When it's not passed the default behaviour would remain the same, but when it is the behaviour needs to be explicitly defined. One option is to just get the explicit list of access levels to export (something like `-export=internal,public,open`) or export levels to be defined (0 for app targets, 1 for public framework targets and 2 for the internal header framework targets for example)

I hope this feature proposal is complete enough to properly define the scope and discuss the viability of a possible solution. Otherwise please let me know.

Thanks.

--
Slds,

Gonzalo.
_______________________________________________
swift-dev mailing list
swift-dev@swift.org <mailto:swift-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-dev

_______________________________________________
swift-dev mailing list
swift-dev@swift.org <mailto:swift-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-dev