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

extra::json: use a different encoding for enums. #8437

Closed
wants to merge 3 commits into from
Closed

extra::json: use a different encoding for enums. #8437

wants to merge 3 commits into from

Conversation

emberian
Copy link
Member

It now uses {"type": VariantName, "fields": [...]}, which, according to
@Seldaek, since all enums will have the same "shape" rather than being a weird
ad-hoc array, will optimize better in javascript JITs. It also looks prettier,
and makes more sense.

It now uses `{"type": VariantName, "fields": [...]}`, which, according to
@Seldaek, since all enums will have the same "shape" rather than being a weird
ad-hoc array, will optimize better in javascript JITs. It also looks prettier,
and makes more sense.
@emberian
Copy link
Member Author

I'm making the PR because it works, but I can't get the tests to pass. It's doing some weird stuff that I don't really understand. git blame tells me to bother @pcwalton and @erickt about it.

@emberian
Copy link
Member Author

Well, it mostly works, that is.

@@ -1522,7 +1533,7 @@ mod tests {
let mut encoder = Encoder(wr);
animal.encode(&mut encoder);
},
~"[\"Frog\",\"Henry\",349]"
~"{\"type\":\"Frog\",\"fields\":[\"Henry\",349]}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still says type here and below

@catamorphism
Copy link
Contributor

@cmr Needs a rebase.

@emberian
Copy link
Member Author

I'm just going to close this for now since it's broke and I don't have the motivation to figure out why.

@emberian emberian closed this Aug 17, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 20, 2022
Replace manual let else patterns with let else

Clears the codebase from places where the lint added by rust-lang#8437 is firing, by adopting let else.

changelog: none
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 21, 2022
Add lint to tell about let else pattern

Adds a lint to tell the user if the let_else pattern should be used.

~~The PR is blocked probably on rustfmt support, as clippy shouldn't suggest features that aren't yet fully supported by all tools.~~ Edit: I guess adding it as a restriction lint for now is the best option, it can be turned into a style lint later.

---

changelog: addition of a new lint to check for manual `let else`
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