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

Improve rustc_mir_build::matches docs #81364

Merged
merged 2 commits into from
Feb 1, 2021
Merged

Improve rustc_mir_build::matches docs #81364

merged 2 commits into from
Feb 1, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Jan 25, 2021

  • Fix typos
  • Add more information
  • General cleanup

@camelid
Improve rustc_mir_build::matches docs
496836a 
- Fix typos
- Add more information
- General cleanup
@camelid camelid added A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-patterns Relating to patterns and pattern matching 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 Jan 25, 2021
@rust-highfive
Copy link
Collaborator

r? @varkor

(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 Jan 25, 2021
@camelid camelid added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jan 25, 2021
camelid
camelid commented Jan 25, 2021
View reviewed changes
compiler/rustc_mir_build/src/build/matches/mod.rs Outdated
@@ -1214,31 +1225,35 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// This is the most subtle part of the matching algorithm. At
/// this point, the input candidates have been fully simplified,
/// and so we know that all remaining match-pairs require some
/// sort of test. To decide what test to do, we take the highest
/// sort of test. To decide what test to perform, we take the highest
/// priority candidate (last one in the list) and extract the
Copy link
Member Author

Choose a reason for hiding this comment

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

cc https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/match.20candidate.20priority.20order

Copy link
Member

Choose a reason for hiding this comment

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

I think this comment is wrong, as in the code itself we see the highest priority candidate being referred to as the first in the list:

rust/compiler/rustc_mir_build/src/build/matches/mod.rs

Lines 1334 to 1335 in 2ea320a

// extract the match-pair from the highest priority candidate
let match_pair = &candidates.first().unwrap().match_pairs[0];

This also seems to match all other references to the sorting order.

@varkor
Copy link
Member

varkor commented Jan 31, 2021

This looks good to me; sorry for the delay. r=me after correcting the comment about the order that you highlighted.

@camelid
Copy link
Member Author

camelid commented Jan 31, 2021

@bors r=varkor

My first @bors r= :)

@bors
Copy link
Contributor

bors commented Jan 31, 2021

📌 Commit 8b52cdc has been approved by varkor

@bors
Copy link
Contributor

bors commented Jan 31, 2021

💡 This pull request was already approved, no need to approve it again.

@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 Jan 31, 2021
@bors
Copy link
Contributor

bors commented Jan 31, 2021

📌 Commit 8b52cdc has been approved by ```

@camelid
Copy link
Member Author

camelid commented Jan 31, 2021

Weird... Maybe I can't put stuff after the r=? @bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2021
@camelid
Copy link
Member Author

camelid commented Jan 31, 2021

@bors r=varkor

@bors
Copy link
Contributor

bors commented Jan 31, 2021

📌 Commit 8b52cdc has been approved by varkor

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 31, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Feb 1, 2021
@JohnTitor
Rollup merge of rust-lang#81364 - camelid:improve-build-matches-docs,…
0be141c 
… r=varkor

Improve `rustc_mir_build::matches` docs

- Fix typos
- Add more information
- General cleanup
@JohnTitor JohnTitor mentioned this pull request Feb 1, 2021
Closed
henryboisdequin pushed a commit to henryboisdequin/rust that referenced this pull request Feb 1, 2021
@henrytheswimmer
Rollup merge of rust-lang#81364 - camelid:improve-build-matches-docs,…
40159aa 
… r=varkor

Improve `rustc_mir_build::matches` docs

- Fix typos
- Add more information
- General cleanup
@henryboisdequin henryboisdequin mentioned this pull request Feb 1, 2021
Closed
@jonas-schievink jonas-schievink mentioned this pull request Feb 1, 2021
Closed
@jonas-schievink jonas-schievink mentioned this pull request Feb 1, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 1, 2021
@bors
Auto merge of rust-lang#81625 - jonas-schievink:rollup-mshpp2n, r=jon…
02b85d7 
…as-schievink

Rollup of 12 pull requests

Successful merges:

 - rust-lang#78641 (Let io::copy reuse BufWriter buffers)
 - rust-lang#79291 (Add error message for private fn)
 - rust-lang#81364 (Improve `rustc_mir_build::matches` docs)
 - rust-lang#81387 (Move some tests to more reasonable directories - 3)
 - rust-lang#81463 (Rename NLL* to Nll* accordingly to C-CASE)
 - rust-lang#81504 (Suggest accessing field when appropriate)
 - rust-lang#81529 (Fix invalid camel case suggestion involving unicode idents)
 - rust-lang#81536 (Indicate both start and end of pass RSS in time-passes output)
 - rust-lang#81592 (Rustdoc UI fixes)
 - rust-lang#81594 (Avoid building LLVM just for llvm-dwp)
 - rust-lang#81598 (Fix calling convention for CRT startup)
 - rust-lang#81618 (Sync rustc_codegen_cranelift)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ab74346 into rust-lang:master Feb 1, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 1, 2021
@camelid camelid deleted the improve-build-matches-docs branch February 1, 2021 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@varkor varkor varkor left review comments

Assignees

@varkor varkor

Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-patterns Relating to patterns and pattern matching C-enhancement Category: An issue proposing an enhancement or a PR with one. 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.51.0
Development

Successfully merging this pull request may close these issues.
5 participants
@camelid @rust-highfive @varkor @bors @rustbot