Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use of pow in separateFractionalSecond #260

Closed
wants to merge 1 commit into from

Conversation

asuhan
Copy link

@asuhan asuhan commented May 20, 2020

Fixes the Yams/Representer.swift:130:21: error: static member 'pow' cannot be used on instance of type 'Double' we hit.

@asuhan asuhan force-pushed the master branch 2 times, most recently from d4f4aa8 to 9012cdc Compare May 20, 2020 00:40
@asuhan asuhan marked this pull request as draft May 20, 2020 00:45
@jpsim
Copy link
Owner

jpsim commented May 20, 2020

How are you hitting that error? Is that a compile time error or runtime? This shouldn’t be happening as best I can tell.

@asuhan
Copy link
Author

asuhan commented May 20, 2020

@jpsim It's a compile error, we hit it when building the Swift toolchain on Ubuntu 18.04.

@asuhan asuhan marked this pull request as ready for review May 20, 2020 02:29
@jpsim
Copy link
Owner

jpsim commented May 20, 2020

It's a compile error, we hit it when building the Swift toolchain on Ubuntu 18.04.

That's odd. Can you please share steps to reproduce the issue? I'm not seeing that on Ubuntu 18.04 with Swift 5.2.2 (also tested with Swift 5.2.3):

$ docker run -it --rm swift:5.2.2 bash 
root@d25a41d005ed:/# cat /etc/issue
Ubuntu 18.04.4 LTS \n \l

root@d25a41d005ed:/# git clone https://github.com/jpsim/Yams.git
Cloning into 'Yams'...
remote: Enumerating objects: 11, done.
remote: Counting objects: 100% (11/11), done.
remote: Compressing objects: 100% (11/11), done.
remote: Total 4869 (delta 0), reused 6 (delta 0), pack-reused 4858
Receiving objects: 100% (4869/4869), 6.59 MiB | 6.84 MiB/s, done.
Resolving deltas: 100% (2865/2865), done.
root@d25a41d005ed:/# cd Yams
root@d25a41d005ed:/Yams# swift build
[22/22] Merging module Yams
root@d25a41d005ed:/Yams# echo $?
0
root@d25a41d005ed:/Yams#

@asuhan
Copy link
Author

asuhan commented May 20, 2020

@asuhan
Copy link
Author

asuhan commented May 20, 2020

Looks like we're not getting the right pow, it works if I explicitly qualify it with Glibc. @compnerd might now more about this.

@compnerd
Copy link
Contributor

@asuhan if you qualify, you need to qualify for multiple targets, but of course it would work when qualified.

The problem here is that pow is not a real function. The functions are powf, powl, and powll. pow is a swift extension to add a type generic function a la C11. The qualified name is exactly that and is why it works.

This might just be a race condition: import System/Glibc/MSVCT and it should work. The typed function is just directly going to the underlying libc function and is cheaper though.

@asuhan
Copy link
Author

asuhan commented May 20, 2020

@compnerd I've already tried to import System/Glibc/MSVCT and didn't get it to work. If I convert to powl, I get:

yams/Sources/Yams/Representer.swift:137:32: error: cannot convert value of type 'Double' to expected argument type 'Float80'
        let radix = powl(10.0, Double(precision))
                               ^
                               Float80(         )

If I don't, I get:

swift-source/yams/Sources/Yams/Representer.swift:137:21: error: static member 'pow' cannot be used on instance of type 'Double'
        let radix = pow(10.0, Double(precision))
                    ^~~
                    Double.

Also, note that I'm only qualifying it for Linux in the current version of the patch, other targets appear to be happy with it.

@jpsim
Copy link
Owner

jpsim commented May 20, 2020

@asuhan could you please file a bug at https://bugs.swift.org? I'd consider it a Swift bug if pow(10.0, Double(1.0)) fails to compile on any platform.

@asuhan
Copy link
Author

asuhan commented May 28, 2020

Turns out the issue is specific to the TF branch: asuhan#1. Sorry!

@asuhan asuhan closed this May 28, 2020
@jpsim
Copy link
Owner

jpsim commented May 28, 2020

I’m glad you got to the bottom of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants