Deprecating M_PI and a few other constants in favor of better API #6764
Conversation
Deprecating `M_PI`, `M_PI_2`, `M_PI_4`, `M_SQRT2`, `M_SQRT1_2`. Sugesting using `Double.pi` or even just `.pi` to allow type be inferred and avoid a type-cast. Fixes: <rdar://problem/26602190>
| public let M_PI_4 = Double.pi / 4 | ||
|
|
||
| @available(swift, deprecated: 3.0, message: "Please use '2.squareRoot()'.") | ||
| public let M_SQRT2 = 2.0.squareRoot() |
There was a problem hiding this comment.
Will LLVM constant-fold this?
There was a problem hiding this comment.
Ah! Good question. It does not. /cc @stephentyrone
There was a problem hiding this comment.
I'm somewhat surprised that LLVM doesn't manage to constant-fold it. We should figure out why not.
There was a problem hiding this comment.
I think it's not LLVM's fault, but Swift's. According to Roman we don't currently lower sqrt into an llvm intrinsic, so LLVM has no chances.
There was a problem hiding this comment.
LLVM can constant-fold the C sqrt, not only llvm.sqrt, can't it? Does squareRoot boil down to a call to sqrt?
There was a problem hiding this comment.
Digging into it, I originally had a static inline wrapper around C sqrt. However, that ran into https://bugs.swift.org/browse/SR-2089, so it had to be replaced with a non-inline stub [some history here: https://github.com//pull/3454]. We should be able to fix that now that 2089 is addressed, which should also let us constant fold these, I think.
There was a problem hiding this comment.
I'll prep a pull request with sqrt inlined again and see where we are.
There was a problem hiding this comment.
@stephentyrone Please mention me on the PR when it's ready.
There was a problem hiding this comment.
Ah... nevermind. Reading email from the bottom. Found it already =)
|
@swift-ci Please test |
|
As to the practical matter of this pull, what you have for the |
|
For the record: I filed rdar://problem/30003973 and rdar://problem/30004033 to track the rest of the work. |
|
Yay, thanks! |
ghost
mentioned this pull request
Deprecating
M_PI,M_PI_2,M_PI_4,M_SQRT2,M_SQRT1_2.Sugesting using
Double.pior even just.pito allow type be inferredand avoid a type-cast.
Fixes: rdar://problem/26602190