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

Clean up and streamline the pretty-printer #56336

Merged
merged 4 commits into from Nov 30, 2018

Conversation

@nnethercote
Copy link
Contributor

commented Nov 29, 2018

Some minor improvements.

nnethercote added some commits Nov 28, 2018

Fix whitespace in `pp.rs`.
This commit converts some 2-space indents to 4-space indents.
Remove `huge_word` and `zero_word`.
They are unused. The commit also adds some blank lines between some
methods.
Use `Cow` in `Token::String`.
`Printer::word` takes a `&str` and converts it into a `String`, which
causes an allocation. But that allocation is rarely necessary, because
`&str` is almost always a `&'static str` or a `String` that won't be
used again.

This commit changes `Token::String` so it holds a `Cow<'static, str>`
instead of a `String`, which avoids a lot of allocations.
Split up `pretty_print` and `print`.
`pretty_print` takes a `Token` and `match`es on it. But the particular
`Token` kind is known at each call site, so this commit splits it into
five functions: `pretty_print_eof`, `pretty_print_begin`, etc.

This commit also does likewise with `print`, though there is one
callsite for `print` where the `Token` kind isn't known, so a generic
`print` has to stay (but it now just calls out to the various `print_*`
functions).
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Nov 29, 2018

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

} else {
self.advance_right();
Token::Eof => {
if !self.scan_stack.is_empty() {

This comment has been minimized.

Copy link
@nikomatsakis

nikomatsakis Nov 29, 2018

Contributor

ah, 2 space indents. Blast from the past.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Nov 29, 2018

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Nov 29, 2018

📌 Commit 64cd645 has been approved by nikomatsakis

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

commented Nov 30, 2018

@bors rollup

kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018

Rollup merge of rust-lang#56336 - nnethercote:clean-up-pp, r=nikomats…
…akis

Clean up and streamline the pretty-printer

Some minor improvements.

bors added a commit that referenced this pull request Nov 30, 2018

Auto merge of #56376 - kennytm:rollup, r=kennytm
Rollup of 16 pull requests

Successful merges:

 - #55821 (Use sort_by_cached_key when the key function is not trivial/free)
 - #56014 (add test for issue #21335)
 - #56131 (Assorted tweaks)
 - #56216 (Add TryFrom<&[T]> for [T; $N] where T: Copy)
 - #56224 (Update cargo)
 - #56268 (Reuse the `P` in `InvocationCollector::fold_{,opt_}expr`.)
 - #56285 (move stage0.txt to toplevel directory)
 - #56305 (update miri)
 - #56336 (Clean up and streamline the pretty-printer)
 - #56339 (Remove not used option)
 - #56341 (Rename conversion util; remove duplicate util in librustc_codegen_llvm.)
 - #56349 (rustc 1.30.0's linker flavor inference is a non-backwards compat change to -Clinker)
 - #56355 (Add inline attributes and add unit to CommonTypes)
 - #56360 (Optimize local linkchecker program)
 - #56364 (Fix panic with outlives in existential type)
 - #56373 (Update books)

Failed merges:

r? @ghost

kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018

Rollup merge of rust-lang#56336 - nnethercote:clean-up-pp, r=nikomats…
…akis

Clean up and streamline the pretty-printer

Some minor improvements.

kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018

Rollup merge of rust-lang#56336 - nnethercote:clean-up-pp, r=nikomats…
…akis

Clean up and streamline the pretty-printer

Some minor improvements.

kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018

Rollup merge of rust-lang#56336 - nnethercote:clean-up-pp, r=nikomats…
…akis

Clean up and streamline the pretty-printer

Some minor improvements.

bors added a commit that referenced this pull request Nov 30, 2018

Auto merge of #56381 - kennytm:rollup, r=kennytm
Rollup of 19 pull requests

Successful merges:

 - #55011 (Add libstd Cargo feature "panic_immediate_abort")
 - #55821 (Use sort_by_cached_key when the key function is not trivial/free)
 - #56014 (add test for issue #21335)
 - #56131 (Assorted tweaks)
 - #56214 (Implement chalk unification routines)
 - #56216 (Add TryFrom<&[T]> for [T; $N] where T: Copy)
 - #56268 (Reuse the `P` in `InvocationCollector::fold_{,opt_}expr`.)
 - #56324 (Use raw_entry for more efficient interning)
 - #56336 (Clean up and streamline the pretty-printer)
 - #56337 (Fix const_fn ICE with non-const function pointer)
 - #56339 (Remove not used option)
 - #56341 (Rename conversion util; remove duplicate util in librustc_codegen_llvm.)
 - #56349 (rustc 1.30.0's linker flavor inference is a non-backwards compat change to -Clinker)
 - #56355 (Add inline attributes and add unit to CommonTypes)
 - #56360 (Optimize local linkchecker program)
 - #56364 (Fix panic with outlives in existential type)
 - #56365 (Stabilize self_struct_ctor feature.)
 - #56367 (Moved some feature gate tests to correct location)
 - #56373 (Update books)
@bors

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2018

☔️ The latest upstream changes (presumably #56381) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors merged commit 64cd645 into rust-lang:master Nov 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nnethercote nnethercote deleted the nnethercote:clean-up-pp branch Nov 30, 2018


pub fn get(&self) -> &'static str {
self.string
}

This comment has been minimized.

Copy link
@Zoxc

Zoxc Mar 16, 2019

Contributor

@eddyb @nnethercote This function is unsound and should be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.