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 never type fallback in !'s docs #124419

Merged
merged 5 commits into from
May 2, 2024
Merged

Document never type fallback in !'s docs #124419

merged 5 commits into from
May 2, 2024

Conversation

WaffleLapkin
Copy link
Member

Pulled the documentation I've written for #123939.

I want a single place where never type fallback is explained, which can be referred in all the lints and migration materials.

@rustbot
Copy link
Collaborator

rustbot commented Apr 26, 2024

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 26, 2024
workingjubilee
workingjubilee reviewed Apr 26, 2024
View reviewed changes
Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

Thanks! Some minor clarity-related nits. Overall looks good to me.

library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
@traviscross traviscross mentioned this pull request Apr 26, 2024
Closed
8 tasks
@WaffleLapkin @workingjubilee
Apply suggestions from code review
b73bfd2 
Co-authored-by: Jubilee <46493976+workingjubilee@users.noreply.github.com>
@workingjubilee
Copy link
Member

nice!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 26, 2024

📌 Commit b73bfd2 has been approved by workingjubilee

It is now in the queue for this repository.

@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 Apr 26, 2024
kpreid
kpreid suggested changes Apr 26, 2024
View reviewed changes
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs
/// To prevent such errors, the compiler remembers where it inserted `absurd` calls, and
/// if it can't infer their type, it sets the type to the fallback type:
/// ```rust, ignore
/// type Fallback = /* An arbitrarily selected type! */;
Copy link
Contributor

Choose a reason for hiding this comment

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

"Arbitrarily selected" here sounds like "the compiler picks from some set", not "we made a choice and taught it to the compiler". I'm not sure what would be better, though.

Copy link
Member

Choose a reason for hiding this comment

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

There's nothing stopping us from having arbitrarily complicated fallback logic, aside from a reluctance to implement anything more complex, which is why I suggested it that way.

We do pick from a set.

The set has one member. :^)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hoping to grow the set to two :p

I'm also not a fan of "arbitrary" here because we did purposefully choose the fallback type...

But I'm not sure how to phrase this properly :/

Copy link
Member

@workingjubilee workingjubilee Apr 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Whenever I use arbitrary I strongly mean the "the reason for why is because I said so", because often there were multiple reasonable choices we could have made, but we could only make one (one set, with any number of members).

library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
@WaffleLapkin
Copy link
Member Author

@bors r-
(so that I can apply @kpreid suggestions)

@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 Apr 26, 2024
jongiddy
jongiddy reviewed Apr 27, 2024
View reviewed changes
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
library/core/src/primitive_docs.rs Outdated Show resolved Hide resolved
WaffleLapkin and others added 2 commits May 2, 2024 04:13
@WaffleLapkin @kpreid
@skogseth @workingjubilee
Apply suggestions from code review
3a40e83 
Co-authored-by: Kevin Reid <kpreid@switchb.org>
Co-authored-by: Herman Skogseth <herman.skogseth@me.com>
Co-authored-by: Jubilee <46493976+workingjubilee@users.noreply.github.com>
@WaffleLapkin
Slightly reformat !'s docs after applying github suggestions
e2eb053
@WaffleLapkin
Copy link
Member Author

Thanks to everyone who participated in the discussions and suggested improvements to the documentation! <3

@bors r=workingjubilee

@bors
Copy link
Contributor

bors commented May 2, 2024

📌 Commit e2eb053 has been approved by workingjubilee

It is now in the queue for this repository.

@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 May 2, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@WaffleLapkin @workingjubilee
fixup links in never type docs
e18b6e8 
Co-authored-by: Jubilee <46493976+workingjubilee@users.noreply.github.com>
@workingjubilee
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 2, 2024

📌 Commit e18b6e8 has been approved by workingjubilee

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 2, 2024

⌛ Testing commit e18b6e8 with merge 80451a4...

@bors
Copy link
Contributor

bors commented May 2, 2024

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing 80451a4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 2, 2024
@bors bors merged commit 80451a4 into rust-lang:master May 2, 2024
7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 2, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (80451a4): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 1.2% [1.2%, 1.2%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.1% [-0.1%, -0.1%] 1
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.5% [-0.1%, 1.2%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 675.937s -> 674.735s (-0.18%)
Artifact size: 315.99 MiB -> 316.01 MiB (0.01%)

@WaffleLapkin WaffleLapkin deleted the never-type-fallback-docs branch May 2, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skogseth skogseth skogseth left review comments

@workingjubilee workingjubilee workingjubilee left review comments

@jongiddy jongiddy jongiddy left review comments

@kpreid kpreid kpreid requested changes

Assignees

@workingjubilee workingjubilee

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.80.0
Development

Successfully merging this pull request may close these issues.
9 participants
@WaffleLapkin @rustbot @workingjubilee @bors @rust-log-analyzer @rust-timer @kpreid @jongiddy @skogseth