add @discardableResult to changeCurrentDirectoryPath(_:)

[jpsim]

To match Darwin's API.

[parkera]

Hm, is the importer applying discardableResult automatically to some set of methods that come from Objective-C? I don't see the annotation in the generated interface, and we don't have anything specifically about this in our API notes.

[jpsim]

I don't know how those internals are set up, but I can say that I wasn't getting a warning in Xcode 8.1 and I started getting warnings on Linux: realm/SwiftLint@31814c3

[briancroom]

Unless there were later policy changes I'm not aware of, @discardableResult is applied pretty globally to imported declarations. See:

https://github.com/apple/swift-evolution/blob/master/proposals/0047-nonvoid-warn.md#default-migration-behavior

[parkera]

So I guess the question I have is this: should we be putting @discardableResult on pretty much everything?

[briancroom]

And/or start considering where it might make sense to annotate some Darwin Foundation declarations with warn_unused_result?

@jrose-apple any thoughts on how Corelibs should approach this?

XCTest doesn't have much API that's affected by this, but we did add @discardableResult in a few places when the keyword was introduced.

[jrose-apple]

I'm not sure. For a new library that had both Objective-C and Swift interfaces, of course we'd recommend adding warn_unused_result where relevant instead of the other way around. (If we weren't just recommending sticking to one language for some reason.) But adding those to Foundation would cause new warnings in existing code, so it's unclear (and up to the Foundation team to decide) whether it's worth dealing with that.

The next step is whether it's okay to diverge, since anyone writing against Swift Foundation ought to be following Swift rules for return values. In that case it'd be a good idea to take changes like this one, where the return value really is unimportant a lot of the time, but not do the mass change. I think the corelibs have made cross-platform compatibility a very high goal, however.

[parkera]

Divergence is not really an option for us; the goal is cross-platform compatibility.

That does not mean that it cannot change, however. We would just need to do it in tandem with a change to the system Foundation.

[jpsim]

@parkera what would you like to do here? Are you ok with PRs that address these inconsistencies on an ad-hoc/piecemeal basis, or do you thinking waiting for a wholesale realignment of all affected APIs is preferable? Also see #740 for another targeted fix.

[parkera]

Let's do them in reasonably sized chunks, so they are easier to merge and less likely to conflict with other PRs in flight.

[jpsim]

I can't tell from your comment if "reasonably sized chunks" means smaller or larger chunks 😉

Given that you merged #740 but not this one, I'm even more confused....

What would you like me to do with this PR to get it merged?

[parkera]

Sorry, I can merge this one too. By reasonably sized I meant one subsystem or so at a time.

[parkera]

@swift-ci test and merge