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

Consider adding JSONCompatible protocol #21

Open
lilyball opened this issue Feb 23, 2018 · 2 comments
Open

Consider adding JSONCompatible protocol #21

lilyball opened this issue Feb 23, 2018 · 2 comments

Comments

@lilyball
Copy link
Collaborator

We should consider having a JSONCompatible protocol that is conformed to by things like String, Int, etc, as a replacement for the family of JSON.init(_:) initializers. We could then have a single JSON.init(_:) initializer that is generic using this protocol.

The goal here is to sidestep the type-checking complexity issue when using a bunch of JSON(foo) calls in the same expression.

We should also do some microbenchmarks on performance, because generics are implemented using a dictionary-passing mechanism, and we want to make sure that switching to a single generic initializer doesn't have a significant effect on how long it takes to construct these.

Note that if we do this we might have to bump the major version in order to mark the existing JSON.init(_:) methods as unavailable. The problem is that just leaving them as deprecated is still likely going to incur the type-checking penalty. But we should test.

@lilyball
Copy link
Collaborator Author

We could also investigate switching the ExpressibleByDictionaryLiteral and ExpressibleByArrayLiteral implementations over to using JSONCompatible existentials, so that way you can mix non-JSON values into the literals, but if so we definitely need to microbenchmark it because this will wrap even JSON values up in protocol existentials which seems like a lot of unnecessary work.

@lilyball lilyball added this to the v4.0 milestone Nov 14, 2019
@lilyball
Copy link
Collaborator Author

I'm going to push this off as well. It's not clear how much of a win there is with this.

@lilyball lilyball removed this from the v4.0 milestone Nov 15, 2019
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

No branches or pull requests

1 participant