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.

Sign up for GitHub

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

Jump to bottom

Json method cleanup #12829

Closed
zslayton opened this issue Mar 11, 2014 · 0 comments
Closed

Json method cleanup #12829

zslayton opened this issue Mar 11, 2014 · 0 comments

Comments

@zslayton
Copy link
Contributor

cc @alexcrichton, @arjantop

The new convenience methods on the Json class could be cleaned up a little bit:

  • All of the methods use the corresponding Json enum variant in their name except for is_str and as_str, which refer to the Rust str type. These should be made is_string and as_string.
  • Implement the is_* methods in terms of the as_* methods.
  • find_path uses a fold/map implementation that does not short circuit in the event of an invalid path. If a path with 10 keys is provided but the first key does not exist, it will still go through the effort of generating all 10 Nones.

I'll take care of these, I just wanted to post them here in case people had other comments.
@zslayton zslayton mentioned this issue Mar 11, 2014
Merged
@zslayton zslayton mentioned this issue Mar 12, 2014
Closed
zslayton added a commit to zslayton/rust that referenced this issue Mar 12, 2014
@zslayton
Closes rust-lang#12829. Names changed for consistency, find_path opti…
b828ba9 
…mized, method impls refactored to reduce repitition.

Fixed formatting, reworked find_path to use fewer Options.

Removed stray tab.
fasterthanlime pushed a commit to fasterthanlime/rust that referenced this issue Jul 22, 2022
@bors
Auto merge of rust-lang#12829 - fasterthanlime:in-rust-tree-feature, …
bd4439f 
…r=Veykril

Remove `check_merge_commits` test

Due to the way "git subtree" works, the `check_merge_commits` test _will_ find merge commits and fail, so we simply skip it.

This changed is tracked in:

  * rust-lang/rust-analyzer#12818

Maintainer impact: none
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 25, 2024
@bors
Auto merge of rust-lang#12830 - blyxyas:more-controlflow, r=y21,xFredNet
16953ce 
Use ControlFlow in more places

Now, instead of manually using variables in visitors to signify that a visit is "done" and that the visitor should stop traversing. We use the trait type "Result" to signify this (in relevant places).

I'll schedule a perf run, I don't think it will be much of a difference, but every bit of performance is welcomed :)

changelog: Improve performance, less memory use in visitors

Fixes rust-lang#12829
r? `@y21`
dingxiangfei2009 pushed a commit to dingxiangfei2009/rust that referenced this issue Jul 28, 2024
@bors
Auto merge of rust-lang#13156 - y21:for_each_expr_visitor_refactor, r…
c7cfe7f 
…=xFrednet

Remove unnecessary `res` field in `for_each_expr` visitors

Small refactor in the `for_each_expr*` visitors. This should not change anything functionally.

Instead of storing the final value `Option<B>` in the visitor and setting it to `Some` when we get a `ControlFlow::Break(B)` from the closure, we can just directly return it from the visitor itself now that visitors support that.

cc rust-lang#12829 and rust-lang/rust-clippy#12830 (comment)

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Json method cleanup zslayton/rust
1 participant
@zslayton