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

Document everybody_loops #82617

Merged
merged 1 commit into from
Mar 14, 2021
Merged

Document everybody_loops #82617

merged 1 commit into from
Mar 14, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Feb 28, 2021

cc @jyn514

@camelid camelid added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Feb 28, 2021
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 28, 2021
@camelid camelid mentioned this pull request Feb 28, 2021
Closed
jyn514
jyn514 reviewed Feb 28, 2021
View reviewed changes
compiler/rustc_interface/src/util.rs
Comment on lines +707 to +727
/// * `const fn`; support could be added, but hasn't. Originally `const fn`
/// was skipped due to issue #43636 that `loop` was not supported for
/// const evaluation.
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to add support here? I think it would be as simple as removing within_static_or_const and https://github.com/rust-lang/rust/blob/bd203dfe0a70bd4c5fe3927b16ed75d3d0e13b2c/compiler/rustc_interface/src/util.rs#L865.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably makes sense to do that in a separate PR since it would require signoff on the change.

compiler/rustc_interface/src/util.rs Outdated
Comment on lines 703 to 722
/// Since [#73566], rustdoc no longer runs `everybody_loops`. As of February 2021,
/// `everybody_loops` is only run for the `-Z unpretty=everybody_loops` CLI option.
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be mentioned? This seems only of historical interest, and you can find it easily with git or a google search.

Copy link
Member Author

Choose a reason for hiding this comment

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

This seems only of historical interest, and you can find it easily with git or a google search.

I wanted to make it clear that it's only used for debugging. Someone reading the code might not think of or might not want to spend the time of doing a search.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, then I would say something like "This is used for the -Z unpretty=everybody_loops debugging pass"`. I don't think you need to mention rustdoc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally, I would still prefer to mention rustdoc, but it doesn't matter that much so I removed the sentence about rustdoc :)

@bors

This comment has been minimized.

Sign in to view
@camelid
Copy link
Member Author

camelid commented Mar 9, 2021

r? @jyn514

@rust-highfive rust-highfive assigned jyn514 and unassigned matthewjasper Mar 9, 2021
@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 10, 2021
@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 13, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 13, 2021

@bors r+

cc @pnkfelix, you might be interested in adding support for const fn back: #82617 (comment)

@bors
Copy link
Contributor

bors commented Mar 13, 2021

📌 Commit 8164a74 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 13, 2021
@JohnTitor JohnTitor mentioned this pull request Mar 14, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2021
@bors
Auto merge of rust-lang#83105 - JohnTitor:rollup-tqpm8pb, r=JohnTitor
1381dcf 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#81465 (Add documentation about formatting `Duration` values)
 - rust-lang#82121 (Implement Extend and FromIterator for OsString)
 - rust-lang#82617 (Document `everybody_loops`)
 - rust-lang#82789 (Get with field index from pattern slice instead of directly indexing)
 - rust-lang#82798 (Rename `rustdoc` to `rustdoc::all`)
 - rust-lang#82804 (std: Fix a bug on the wasm32-wasi target opening files)
 - rust-lang#82943 (Demonstrate best practice for feeding stdin of a child processes)
 - rust-lang#83066 (Add `reverse` search alias for Iterator::rev())
 - rust-lang#83070 (Update cargo)
 - rust-lang#83081 (Fix panic message of `assert_failed_inner`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f0ebc10 into rust-lang:master Mar 14, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 14, 2021
@camelid camelid deleted the everybody_loops-docs branch March 14, 2021 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

Assignees

@jyn514 jyn514

Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.52.0
Development

Successfully merging this pull request may close these issues.
6 participants
@camelid @rust-highfive @bors @jyn514 @matthewjasper @rustbot