Unavail the Darwin.abs function to avoid collisions with Swift.abs

[stephentyrone]

Also marks M_PI etc obsoleted instead of merely deprecated (which they have been since Swift 3.0).

Fixes rdar://problem/44044108

[stephentyrone]

@swift-ci please test

[stephentyrone]

@swift-ci please test source compatibility

[compnerd]

Shouldn't a similar change be done in the Windows SDK overlay?

[stephentyrone]

Shouldn't a similar change be done in the Windows SDK overlay?

Probably, but I know nothing about what APIs Windows does (or should) provide, so I will leave that to someone else.

[jrose-apple]

These versions are language versions, and there is no "Swift 5.1" language version. I'm afraid you can't do this conditionally.

[jrose-apple]

(I mean, you can, but it won't have any effect until we get a -swift-version 6 or something.)

[stephentyrone]

@jrose-apple bad news for the obsoletion of flatMap, too:

@available(swift, deprecated: 4.1, obsoleted: 5.1, renamed: "compactMap(_:)"

Since these are tied to the Darwin overlay, they can plausibly use macOS etc. versions instead, but if they were stdlib features (like flatMap), it seems like we'd want to use obsoleted: 5.1 even if there isn't a language version, to ensure that it goes away in the first version where it is meaningful.

[jrose-apple]

I remember mentioning this there too, and getting the response that it was okay since it would eventually have an effect. That's different from trying to avoid collisions, though.

(Are the collisions actually a problem? I thought we said the standard library is always shadowed by what's in other libraries when there's a perfect overload.)

[stephentyrone]

@jrose-apple The collision is a problem because fabs is deprecated renamed: abs, but if someone has Darwin.fabs, the fixit replaces it it with Darwin.abs and turns a deprecation warning into a compile error (because Darwin.abs has the signature (Int32) -> Int32). A better fix would be the ability to do renamed: Swift.abs and get the right fixit, but we don't need Darwin.abs sitting around anyway.

[stephentyrone]

I thought we said the standard library is always shadowed by what's in other libraries when there's a perfect overload

This is often not ideal for the specific case of the platform overlay. We want the stdlib to be shadowed by user libraries, but in the case of Platform, we are actively trying to provide more of that functionality in the stdlib, so more often than not we would prefer the stdlib versions to get picked up.

[jrose-apple]

The platform overlay currently isn't treated specially by the compiler, and I don't think this would be sufficient reason to do so.

[stephentyrone]

I agree, hence the removals.

[jrose-apple]

…ugh. I misread the patch from the beginning, and thought you were using obsoleted to do the removals. Carry on.

[Coeur]

Please leave it to the human readable form squareRoot instead of the cryptic sqrt name (squirt?) as it's not following the Swift guidelines. Or address this renaming in a separate pull request.

[stephentyrone]

@Coeur sqrt is a sanctioned name of the operation per SE-0246, and per the swift naming guidelines:

Embrace precedent. Don’t optimize terms for the total beginner at the expense of conformance to existing culture.

We'll have both squareRoot and sqrt going forward, and both are perfectly acceptable. We may eventually deprecate one of them, but that would be another SE proposal. In the case of working with literals, the free function sqrt(2) reads much more naturally than the method (2.0).squareRoot().

[Coeur]

@stephentyrone well, I just noticed that you did replace _swift_stdlib_squareRootf with _swift_stdlib_sqrtf some three years ago, and you did ask some 2 years ago if sqrt(2) wasn't better than 2.squareRoot(). So you're clearly more experienced than I am on the naming here. But while there are two forms currently acceptable, I don't see what would be the benefit to flip the recommendation here.

[stephentyrone]

@Coeur What changed is SE-0246. Prior to that the only spelling provided by stdlib was .squareRoot() (sqrt(x) came from the platform overlays). We have always intended to expose a global free function as well, but hadn't decided how it would be spelled in the standard library, so I didn't want to recommend something that might not be the real name. We now know what the spelling of that free function is, and can recommend using it.

[Coeur]

OK, I've just read the proposal SE-0246 and the review, and nobody ever mentioned squareRoot once. :(
Too bad, but oh well, not a big issue. I just left them a comment for posterity.

[stephentyrone]

We're punting on this until we have an actual language version to tie it to.