Skip to content

Conversation

@weissi
Copy link
Member

@weissi weissi commented May 16, 2018

Motivation:

Minus SR-7578 which will be
fixed soon, we should then compile warning-free in -swift-version 4
mode which is good.
However, we should also be as warning-free as possible in
-swift-version 4.2. This is not entirely possible because of
@_inlineable vs @inlinable but we can address the other warnings.

Modifications:

  • make CountableRange vs Range (which used to be separate types but
    are now the same) work warning-free Swift 4.[01] and Swift 4.2

Result:

Fewer warnings.

test this the following way:

# make sure you have a Swift 4.2 compiler installed as `swift`
rm -rf .build && swift build -Xswiftc -swift-version -Xswiftc 4.2 2>&1 | grep warning: | grep -v -e "'@_versioned' has been renamed to '@usableFromInline'" -e "'@_inlineable' has been renamed to '@inlinable'"

@weissi weissi requested review from Lukasa and normanmaurer and removed request for Lukasa May 16, 2018 15:42
@weissi weissi added the 🔨 semver/patch No public API change. label May 16, 2018
@weissi weissi added this to the 1.7.0 milestone May 16, 2018
Motivation:

Minus [SR-7578](https://bugs.swift.org/browse/SR-7578) which will be
fixed soon, we should then compile warning-free in `-swift-version 4`
mode which is good.
However, we should also be as warning-free as possible in
`-swift-version 4.2`. This is not entirely possible because of
`@_inlineable` vs `@inlinable` but we can address the other warnings.

Modifications:

- make `CountableRange` vs `Range` (which used to be separate types but
  are now the same) work warning-free Swift 4.[01] and Swift 4.2

Result:

Fewer warnings.
@weissi weissi force-pushed the jw-4.2-in-4.2-mode branch from 005b49b to 822a3cf Compare May 16, 2018 16:20
@weissi
Copy link
Member Author

weissi commented May 16, 2018

@normanmaurer ok, turns out we need both @_inlineable and @_versioned because before Swift 4.2, @_inlineable can only be applied to public functions so we need @_versioned and @_inlineable for the internal ones :'(

Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

LGTM

@normanmaurer
Copy link
Member

@weissi before merging please adjust commit message as it still mention @_versioned etc.

@weissi
Copy link
Member Author

weissi commented May 17, 2018

@normanmaurer thanks, fixed. Had removed it from the commit msg but not from the PR describe. Now done too

@weissi weissi merged commit 6020265 into apple:master May 17, 2018
@weissi weissi deleted the jw-4.2-in-4.2-mode branch May 17, 2018 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants