[Review] SE-0075: Adding a Build Configuration Import Test


(Chris Lattner) #1

Hello Swift community,

The review of "SE-0075: Adding a Build Configuration Import Test" begins now and runs through May 16. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0075-import-test.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager


(Xiaodi Wu) #2

        * What is your evaluation of the proposal?

+1, solves a useful problem on the horizon.

        * Is the problem being addressed significant enough to warrant a
change to Swift?

Yes.

        * Does this proposal fit well with the feel and direction of Swift?

Yes. Although, if I could paint the bike shed just a little, "importable"
feels like a more Swifty term than "canImport".

        * If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?

I haven't.

        * How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?

Followed initial discussion; re-read proposal.


(David Hart) #3

  * What is your evaluation of the proposal?

I think the proposal is worthwhile and I see myself using those when writing multiplatform code. I agree that they are much less brittle than os tests.

  * Is the problem being addressed significant enough to warrant a change to Swift?

Yes.

  * Does this proposal fit well with the feel and direction of Swift?

Yes.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

No. But I think that Swift will have many more non-multi-platform modules due to its roots.

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A quick read.


#4

Hello Swift community,

The review of "SE-0075: Adding a Build Configuration Import Test" begins now and runs through May 16. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0075-import-test.md

* What is your evaluation of the proposal?

The goal is valid, but I do have an issue with the 'canImport'. In my world it's not because you can import something that you want to use it. So for conditional compile code, I see this more as a 'didImport'.

Unfortunately for the conditional import, since I'm on the page of it's not because you can, that you should; I think it better serve by checking against the os() or arch(). Though, there is a need to ask to import something that might not exist; will this require both 'canImport' and 'didImport', or should we, along my 'didImport', include a 'weakImport/quietImport/importIfExist' which ignores failure to import?

* Is the problem being addressed significant enough to warrant a change to Swift?

With common source there's a need to be able to compile code against different API providing the same functionality, so something like this is needed

* Does this proposal fit well with the feel and direction of Swift?

* If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Followed, the initial discussion, but not recalling much about it.

Dany

···

On May 10, 2016, at 2:49 PM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Krzysztof Siejkowski) #5

* What is your evaluation of the proposal?
+1.

It attacks a real problem that is met regularly when working on multi-platform code.

I also like `canImport`. As it’s gonna be used mostly after #if, the whole statement becomes very readable.

* Is the problem being addressed significant enough to warrant a change to Swift?
Definitely. Also, now is the good time to introduce such a change, since the multiplatform use of Swift is only getting momentum.

* Does this proposal fit well with the feel and direction of Swift?
I do think so. Swift was always presented with a vision of language of the future, something that can take place of C++ in the terms of the best solution to write multiplatform code. A great community effort has already introduced basic support for Android, RaspberryPi, multiple Linux distros. The differences in frameworks availability and naming are unavoidable.

* If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
No, I haven’t. I’ve been bitten by problems coming from a lack of such a feature in other languages, though.

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
I’ve read the proposal and related discussion.

Cheers,

Krzysztof


(Pyry Jahkola) #6

The review of "SE-0075: Adding a Build Configuration Import Test" begins now and runs through May 16. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0075-import-test.md

  * What is your evaluation of the proposal?

+1, I think it's a welcome change that improves modularity between libraries.

  * Is the problem being addressed significant enough to warrant a change to Swift?

Yes.

There's another real problem that this feature solves which I couldn't see mentioned in the proposal: adding protocol conformances to optional dependencies. Suppose a library named Frobnication defined a protocol `Frobnicatable`, with some known conformances:

    // module Frobnication
    public protocol Frobnicatable {
        mutating func frobnicate()
    }
    extension String : Frobnicatable { ... }
    extension Frob : Frobnicatable { ... }

Now, suppose something in a 3rd party library called Knobs was also clearly `Frobnicatable` but `canImport` didn't exist. The community would have to pick from the following poorly scaling alternatives:

#1) Either Frobnication would require and import Knobs as its dependency and add the conformances,
#2) or vice versa, Knobs would pull Frobnication as its dependency,
#3) or everybody using both libraries would need to define the conformances theirselves,
#4) or someone would need to maintain a "KnobsFrobnication" library for the sole purpose of defining the said protocol conformances.

* * *

With `canImport`, on the other hand, we get two good options:

#5) Either Frobnication checks `#if canImport(Knobs)` and conditionally adds the conformances,
#6) or Knobs checks if it `canImport(Frobnication)` and conditionally adds the conformances to any suitable types it defines (which is what any new libraries that the author of Frobnication wasn't aware of could do in the future).

* * *

But there remains one issue that there's no clear path for moving conformance definitions from one library into the other one day. This proposal could be improved by allowing to check for library versions too! If similarly to `#if swift(>=2.2)`, we could check `#if canImport(Knobs >= @2.10.0)`, then the authors of Knobs and Frobnication could organise an orchestrated move of conformance definitions from one library into another. Before the move, Knobs 2.9.x would have defined:

    // module Knobs 2.9.x
    #if canImport(Frobnication)
    import Frobnication
    extension Knob : Frobnicatable { ... }
    #endif

Preparing for the move, a new version of Frobnication could introduce the conformance thusly:

    // module Frobnication 1.7.3
    #if canImport(Knobs >= @2.10.0) // *) see note below
    import Knobs
    extension Knob : Frobnicatable { ... }
    #endif

Then, Knobs could gracefully sunset its conformance definitions beginning from the 2.10.0 release:

    // module Knobs 2.10.0
    #if canImport(Frobnication < @1.7.3) // <- Only added version constraint here.
    import Frobnication
    extension Knob : Frobnicatable { ... }
    #endif

*) I'm not sold to any specific syntax, but we could e.g. use the `@` prefix to distinguish version number literals like `@1`, `@0.10` and `@0.10.0` from similar but differently behaving numeric literals.

In any case, even with just `canImport(module)`, we can do a lot better than currently.

  * Does this proposal fit well with the feel and direction of Swift?

Yes.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Yes, like mentioned in the proposal, it's very similar to the `__has_import(<path/to/import.h>)` macro in Clang. I'm not fully familiar with the design space in Haskell, but it seems to me instead of using something similar to `canImport` Haskellers tend to favour the approach I listed above as alternative #4 (e.g. https://github.com/lens/lens-aeson/ brings the lens and aeson libraries together).

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Quick reading.

— Pyry


(Erica Sadun) #7

canImport (or whatever it ends up being called) is deliberate.

You test before you import:

#if canImport(x)
    import x
#else
...
#endif

and you test at the use-site

#if canImport(x)
   // use things that are available in x
#else
...

So you don't import UIKit unless you *can*, and you don't use UIColor unless you can import UIKit. This follows closely on the design of __has_include.

-- E

···

On May 12, 2016, at 7:30 PM, Dany St-Amant via swift-evolution <swift-evolution@swift.org> wrote:

On May 10, 2016, at 2:49 PM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0075: Adding a Build Configuration Import Test" begins now and runs through May 16. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0075-import-test.md

* What is your evaluation of the proposal?

The goal is valid, but I do have an issue with the 'canImport'. In my world it's not because you can import something that you want to use it. So for conditional compile code, I see this more as a 'didImport'.

Unfortunately for the conditional import, since I'm on the page of it's not because you can, that you should; I think it better serve by checking against the os() or arch(). Though, there is a need to ask to import something that might not exist; will this require both 'canImport' and 'didImport', or should we, along my 'didImport', include a 'weakImport/quietImport/importIfExist' which ignores failure to import?


(Patrick Smith) #8

canImport (or whatever it ends up being called) is deliberate.

You test before you import:

#if canImport(x)
   import x
#else
...
#endif

and you test at the use-site

#if canImport(x)
  // use things that are available in x
#else
...

So you don't import UIKit unless you *can*, and you don't use UIColor unless you can import UIKit. This follows closely on the design of __has_include.

-- E

I guess one issue I can see is it’s used in two different ways:
- The first use of canImport is used to check whether it can import a module, and then does so, but there’s no requirement for it to do so. Is this the right this to do?
- The second use of canImport makes no guarantee that the module has been imported, only that it can.

What if instead `import` could return whether it imported or not, when used with #if? Instead of ‘can import’, you get ‘did just import’ and ‘has imported’.

import Required // Error if not present, current behaviour

#if import CoolThing // Skips code block if not present, imports otherwise
  // Do something with CoolThing module
#else
  import AlmostAsCoolThing
#endif

and you test at the use-site

#if module(X) // Does not import, only checks if it has been imported
  // use things that are available in X
#else

As per Pyry’s feedback, you could add a version:

#if import Frobnication(<1.7.3) // <- Only added version constraint here.
extension Knob : Frobnicatable { ... }
#endif

Just a way to make it less low level.


(Erica Sadun) #9

canImport (or whatever it ends up being called) is deliberate.

You test before you import:

#if canImport(x)
  import x
#else
...
#endif

and you test at the use-site

#if canImport(x)
// use things that are available in x
#else
...

So you don't import UIKit unless you *can*, and you don't use UIColor unless you can import UIKit. This follows closely on the design of __has_include.

-- E

I guess one issue I can see is it’s used in two different ways:
- The first use of canImport is used to check whether it can import a module, and then does so, but there’s no requirement for it to do so. Is this the right this to do?
- The second use of canImport makes no guarantee that the module has been imported, only that it can.

What if instead `import` could return whether it imported or not, when used with #if? Instead of ‘can import’, you get ‘did just import’ and ‘has imported’.

That would be a much more complicated proposal than this simple build configuration test.

As per Pyry’s feedback, you could add a version:

#if import Frobnication(<1.7.3) // <- Only added version constraint here.
extension Knob : Frobnicatable { ... }
#endif

I have no problem with this but would need to defer to the build and language people to determine whether that's practical in today's Swift. Right now, there's a major-version mention in build packages but I'm not sure whether that information then propagates in a usable way. If it's possible, then yes, I'd rather add it in the initial design than as a later addition and I can extend Pyry's suggestion in "Future Directions".

I've cc'ed in Daniel Dunbar to see if he has anything specific to add about this.

-- E

p.s. Also on my Swift Bucket list: "import as".

···

On May 13, 2016, at 1:42 AM, Patrick Smith <pgwsmith@gmail.com> wrote:


#10

Hello,

`#if import Foo` can not deal with the fact that a single source file may have to perform the importability test several times.

For example:

  #if canImport(UIKit)
    import UIKit
    // Some UIKit-related declarations
  #endif
  // Later in the same file
  func f() {
    #if canImport(UIKit)
      // Use UIKit-only declarations
    #endif
  }

I know, I know, some will tell me to refactor my code. So let's just say I'm prototyping and that the code doesn't have its final shape, OK?

Still, testing for module importability is not the same as importing it.

Gwendal Roué

···

Le 13 mai 2016 à 10:40, Pyry Jahkola via swift-evolution <swift-evolution@swift.org> a écrit :

Patrick,

I think you're making valuable points here. I also can't think of cases where you wouldn't also import a module in case it was found to be importable. So the use cases I can think of could as well be tackled by allowing expressions such as `import Foo.Bar` as compile-time checks within the conditions of `#if` like you suggested. That would bring those libraries only visible within the scope of that block.

However, there can be cases where you're considering importing more than one module, so something like:

    #if import Foo, import Bar
      ...
    #elseif import Baz
      ...
    #endif

should be considered in that design too. And I don't like the fact that it would import many modules in one line of code.

However, I don't get your concerns of "whether already imported or not". Isn't `import` strictly about bringing identifiers of linked libraries visible in the current file and not about linking to libraries in code.

— Pyry

I guess one issue I can see is it’s used in two different ways:
- The first use of canImport is used to check whether it can import a module, and then does so, but there’s no requirement for it to do so. Is this the right this to do?
- The second use of canImport makes no guarantee that the module has been imported, only that it can.

What if instead `import` could return whether it imported or not, when used with #if? Instead of ‘can import’, you get ‘did just import’ and ‘has imported’.

import Required // Error if not present, current behaviour

#if import CoolThing // Skips code block if not present, imports otherwise
// Do something with CoolThing module
#else
import AlmostAsCoolThing
#endif

and you test at the use-site

#if module(X) // Does not import, only checks if it has been imported
// use things that are available in X
#else

As per Pyry’s feedback, you could add a version:

#if import Frobnication(<1.7.3) // <- Only added version constraint here.
extension Knob : Frobnicatable { ... }
#endif

Just a way to make it less low level.

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

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


#11

Neat idea!

Gwendal

···

Le 13 mai 2016 à 11:05, Pyry Jahkola <pyry.jahkola@iki.fi> a écrit :

Gwendal Roué wrote:

`#if import Foo` can not deal with the fact that a single source file may have to perform the importability test several times.

This would be less of a problem if conditional imports like that worked locally in all scopes of code, so you could write just

    func foo() {
        #if import UIKit
            // Actually use UIKit...
        #endif
        // UIKit no longer visible.
    }

— Pyry


(Pyry Jahkola) #12

Patrick,

I think you're making valuable points here. I also can't think of cases where you wouldn't also import a module in case it was found to be importable. So the use cases I can think of could as well be tackled by allowing expressions such as `import Foo.Bar` as compile-time checks within the conditions of `#if` like you suggested. That would bring those libraries only visible within the scope of that block.

However, there can be cases where you're considering importing more than one module, so something like:

    #if import Foo, import Bar
      ...
    #elseif import Baz
      ...
    #endif

should be considered in that design too. And I don't like the fact that it would import many modules in one line of code.

However, I don't get your concerns of "whether already imported or not". Isn't `import` strictly about bringing identifiers of linked libraries visible in the current file and not about linking to libraries in code.

— Pyry

···

I guess one issue I can see is it’s used in two different ways:
- The first use of canImport is used to check whether it can import a module, and then does so, but there’s no requirement for it to do so. Is this the right this to do?
- The second use of canImport makes no guarantee that the module has been imported, only that it can.

What if instead `import` could return whether it imported or not, when used with #if? Instead of ‘can import’, you get ‘did just import’ and ‘has imported’.

import Required // Error if not present, current behaviour

#if import CoolThing // Skips code block if not present, imports otherwise
// Do something with CoolThing module
#else
import AlmostAsCoolThing
#endif

and you test at the use-site

#if module(X) // Does not import, only checks if it has been imported
// use things that are available in X
#else

As per Pyry’s feedback, you could add a version:

#if import Frobnication(<1.7.3) // <- Only added version constraint here.
extension Knob : Frobnicatable { ... }
#endif

Just a way to make it less low level.

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


(Pyry Jahkola) #13

Gwendal Roué wrote:

`#if import Foo` can not deal with the fact that a single source file may have to perform the importability test several times.

This would be less of a problem if conditional imports like that worked locally in all scopes of code, so you could write just

    func foo() {
        #if import UIKit
            // Actually use UIKit...
        #endif
        // UIKit no longer visible.
    }

— Pyry

···

For example:

  #if canImport(UIKit)
    import UIKit
    // Some UIKit-related declarations
  #endif
  // Later in the same file
  func f() {
    #if canImport(UIKit)
      // Use UIKit-only declarations
    #endif
  }

I know, I know, some will tell me to refactor my code. So let's just say I'm prototyping and that the code doesn't have its final shape, OK?

Still, testing for module importability is not the same as importing it.


(Patrick Smith) #14

Well my idea also included module(X), modelled after the os() function, e.g. #if os(OSX)

#if import UIKit
  // Some UIKit-related declarations
#endif
// Later in the same file
func f() {
  #if module(UIKit)
    // Use UIKit-only declarations
  #endif
}

Looking forward to seeing more feedback, esp from Erica. My concern was that hasModule() was just a bit raw.

I will point out a few concerns I have:

Is there a better way of writing this with nothing inside:
#if import UIKit
#endif

Is it strange that all other functions with #if use parentheses (), but not import?

However, I just feel code like this doesn’t feel very Swifty:

#if hasModule(UIKit)
  import UIKit
#endif

However, I don't get your concerns of "whether already imported or not". Isn't `import` strictly about bringing identifiers of linked libraries visible in the current file and not about linking to libraries in code.

I was originally going to include this, but cut it out, because it would be an unclear way to still import something. import ‘returned’ a boolean. So forgot to cut that bit out too.

#if ! import SomethingCool
  import SomeFallback
#endif

···

On 13 May 2016, at 6:54 PM, Gwendal Roué <gwendal.roue@gmail.com> wrote:

Hello,

`#if import Foo` can not deal with the fact that a single source file may have to perform the importability test several times.

For example:

  #if canImport(UIKit)
    import UIKit
    // Some UIKit-related declarations
  #endif
  // Later in the same file
  func f() {
    #if canImport(UIKit)
      // Use UIKit-only declarations
    #endif
  }

I know, I know, some will tell me to refactor my code. So let's just say I'm prototyping and that the code doesn't have its final shape, OK?

Still, testing for module importability is not the same as importing it.

Gwendal Roué

Le 13 mai 2016 à 10:40, Pyry Jahkola via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> a écrit :

Patrick,

I think you're making valuable points here. I also can't think of cases where you wouldn't also import a module in case it was found to be importable. So the use cases I can think of could as well be tackled by allowing expressions such as `import Foo.Bar` as compile-time checks within the conditions of `#if` like you suggested. That would bring those libraries only visible within the scope of that block.

However, there can be cases where you're considering importing more than one module, so something like:

    #if import Foo, import Bar
      ...
    #elseif import Baz
      ...
    #endif

should be considered in that design too. And I don't like the fact that it would import many modules in one line of code.

However, I don't get your concerns of "whether already imported or not". Isn't `import` strictly about bringing identifiers of linked libraries visible in the current file and not about linking to libraries in code.

— Pyry

I guess one issue I can see is it’s used in two different ways:
- The first use of canImport is used to check whether it can import a module, and then does so, but there’s no requirement for it to do so. Is this the right this to do?
- The second use of canImport makes no guarantee that the module has been imported, only that it can.

What if instead `import` could return whether it imported or not, when used with #if? Instead of ‘can import’, you get ‘did just import’ and ‘has imported’.

import Required // Error if not present, current behaviour

#if import CoolThing // Skips code block if not present, imports otherwise
// Do something with CoolThing module
#else
import AlmostAsCoolThing
#endif

and you test at the use-site

#if module(X) // Does not import, only checks if it has been imported
// use things that are available in X
#else

As per Pyry’s feedback, you could add a version:

#if import Frobnication(<1.7.3) // <- Only added version constraint here.
extension Knob : Frobnicatable { ... }
#endif

Just a way to make it less low level.

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

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


(Patrick Smith) #15

I can’t decide if this would be a good idea or not? I can see pluses and minuses!

···

+
Consistent.
One way to remember to work with modules.
Reinforces the rule that if you want to work with a module, you want to import it.

-
Could get confusing exactly where things are being imported, but you can stick an import away from the top of file today.

I imagine might still need something like `#if module(UIKit)` for certain scenarios, but not sure.

Patrick

On 13 May 2016, at 7:05 PM, Pyry Jahkola via swift-evolution <swift-evolution@swift.org> wrote:

This would be less of a problem if conditional imports like that worked locally in all scopes of code, so you could write just

    func foo() {
        #if import UIKit
            // Actually use UIKit...
        #endif
        // UIKit no longer visible.
    }


(Pyry Jahkola) #16

As per Pyry’s feedback, you could add a version:

#if import Frobnication(<1.7.3) // <- Only added version constraint here.
extension Knob : Frobnicatable { ... }
#endif

I have no problem with this but would need to defer to the build and language people to determine whether that's practical in today's Swift. Right now, there's a major-version mention in build packages but I'm not sure whether that information then propagates in a usable way. If it's possible, then yes, I'd rather add it in the initial design than as a later addition and I can extend Pyry's suggestion in "Future Directions".

I already gave my +1 on the original proposal and if `canImport` is indeed easiest to implement we should get it going now.

The `#if import Foo` blocks and conditional imports with version checks can easily be added at a later time without much complication or breakage, AFAICT. Good if you can include those in the "Future Directions" section.

p.s. Also on my Swift Bucket list: "import as".

Splendid! I'd already forgotten about qualified imports and renaming! Those would be welcome additions too.

— Pyry

···

On 13 May 2016, Erica Sadun wrote: