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

Quick fix for PMJSON on Linux #12

Closed
wants to merge 9 commits into from
Closed

Conversation

VolkerBb
Copy link

@VolkerBb VolkerBb commented Mar 6, 2017

I've integrated PMJSON to parse big JSON file on a Kitura project and recognised that PMJSON 2.0.1 wouldn't compile on the ibmcom/swift-ubuntu docker image (for example). On Linux, the DecimalPaceholder object couldn't be compared (thus the == function), a few methods couldn't be called because there are changes to swift that prevent fileprivate or private methods to be called. Lastly I had to add some ! for the strtod call.
It works for me now, maybe you could review the changes or test in your own branch. Thanks for your great work!


This change is Reviewable

@lilyball
Copy link
Collaborator

lilyball commented Mar 6, 2017

Whoa, SIL verification failure for stuff to do with a private or hidden symbol? That's a pretty wacky error. I would really prefer not to make all that stuff public, though, so I'll see if I can figure out an alternative solution.

@lilyball lilyball closed this in c2f0a20 Mar 6, 2017
@lilyball
Copy link
Collaborator

lilyball commented Mar 6, 2017

I rewrote your PR because I didn't want to expose that stuff publicly. The visibility issues were actually issues with @inline(__always) (see SR-3030).

Thanks for the contribution!

@lilyball
Copy link
Collaborator

lilyball commented Mar 6, 2017

I also released v2.0.2 for this fix, so you can target that with SPM.

@VolkerBb
Copy link
Author

VolkerBb commented Mar 7, 2017

Great, thanks Kevin!

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.

None yet

2 participants