Skip to content

Improve rustc_mir_build::matches docs#81364

Merged
bors merged 2 commits intorust-lang:masterfrom
camelid:improve-build-matches-docs
Feb 1, 2021
Merged

Improve rustc_mir_build::matches docs#81364
bors merged 2 commits intorust-lang:masterfrom
camelid:improve-build-matches-docs

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
Contributor

rust-highfive commented Jan 25, 2021

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
/// 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

@camelid camelid Jan 25, 2021

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
Contributor

@varkor varkor Jan 31, 2021

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
Contributor

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
Update outdated comment
8b52cdc 
The highest-priority item is the *first* in the list, not the last.
See [this code][1] for more.

[this code]: https://github.com/rust-lang/rust/blob/0e63af5da3400ace48a0345117980473fd21ad73/compiler/rustc_mir_build/src/build/matches/mod.rs#L1334-L1335
@camelid
Copy link
Member Author

camelid commented Jan 31, 2021

@bors r=varkor

My first @bors r= :)

@bors
Copy link
Collaborator

bors commented Jan 31, 2021

📌 Commit 8b52cdc has been approved by varkor

@bors
Copy link
Collaborator

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
Collaborator

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
Collaborator

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

1 more reviewer

@varkor varkor varkor left review comments

Reviewers whose approvals may not affect merge requirements

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