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

Rollup of 7 pull requests #107472

Merged
merged 22 commits into from
Jan 30, 2023
Merged

Rollup of 7 pull requests #107472

merged 22 commits into from
Jan 30, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

WaffleLapkin and others added 22 commits January 20, 2023 17:40
The comment says that it is called from main.js, but there don't seem to
be any references to it in main.js.

A quick ripgrep says there are no references in all of librustdoc.
The implementation for the ToOwned::clone_into method on [T] is a copy
of the code for vec::clone_from. In 3613980 the code for
vec::clone_from gained a specialization for when T is Copy. This commit
copies that specialization over to the clone_into implementation.
In the past, Vec::clone_from was implemented using slice::clone_into.
The code from clone_into was later duplicated into clone_from in
8725e4c, which is the commit that adds custom allocator support to
Vec. Presumably this was done because the slice::clone_into only works
for vecs with the default allocator so it would have the wrong type to
clone into Vec<T, A>.

Now that the clone_into implementation is moved out into a specializable
trait anyway we might as well use that to share the code between the two
methods.
Prior to this PR, if the page is restored from the browser bfcache¹, we
call `switchToSavedTheme`. But `switchToSavedTheme` never looks at the
`use-system-theme` preference. Further, if it can't find a saved theme,
it will fall back to the default of "light".

For a user with cookies disabled² whose preferred color scheme is dark,
this means the theme will wobble back and forth between dark and light.
The sequence that occurs is,

1. The page is loaded. During a page load, we consult
   `use-system-theme`: as cookies are disabled, this preference is
   unset. The default is true.

   Because the default is true, we look at the preferred color scheme:
   for our example user, that's "dark". **The page theme is set to
   dark.** We'll attempt to store these preferences in localStorage, but
   fail due to cookies being disabled.

2. The user navigates through the docs. Subsequent page loads happen,
   and the same process in step 1 recurs. Previous pages are
   (potentially) put into the bfcache.

3. The user navigates backwards/forwards, causing a page in bfcache to
   be pulled out of cache. The `pageShow` event handler is triggered.
   However, this calls `switchToSavedTheme`: this doesn't consider the
   system theme, as noted above. Instead, it only looks for a saved
   theme. However, with cookies disabled, there is none. It defaults to
   light. **The page theme is set to light!** The user wonders why the
   dark theme is lost.

There are effectively two functions trying to determine and apply the
correct theme: `updateSystemTheme` and `switchToSavedTheme`. Thus, we
merge them into just one: `updateTheme`. This function contains all the
logic for determining the correct theme, and is called in all
circumstances where we need to set the theme:

* The initial page load
* If the browser preferred color scheme (i.e., light/dark mode) is
  changed
* If the page is restored from bfcache
* If the user updates the theme preferences (i.e., in `settings.js`)

Fixes rust-lang#94250.

¹bfcache: https://web.dev/bfcache/ The bfcache is used to sleep a page,
if the user navigates away from it, and to restore it from cache if the
user returns to it.

²Note that the browser preference that enables/disables cookies really
controls many forms of storage. The same preference thus also affects
localStorage. (This is so a normal browser user doesn't need to
understand the distinction between "cookies" and "localStorage".)
The original tweet in the chain linked to, and thus the through line of links back to Anna's tweet where she named the turbofish (https://web.archive.org/web/20210911061514/https://twitter.com/whoisaldeka/status/914914008225816576) are lost as the user whoisaldeka has deleted their twitter account.

Switching to an archive link preserves this through line, allowing someone to browse back to see the point at which Anna created the turbofish, as was the original intent of including this context.
…r_hir_by_the_next_morning, r=Nilstrieb

Add and use expect methods to hir.

[The future has come](https://github.com/rust-lang/rust/pull/106090/files#r1070062462).

r? `@Nilstrieb`

tbh I'm not even sure if it's worth it
Reimplement NormalizeArrayLen based on SsaLocals

Based on rust-lang#106908
Fixes rust-lang#105929

Only the last commit "Reimplement NormalizeArrayLen" is relevant.
…e, r=notriddle

Keep all theme-updating logic together

Prior to this PR, if the page is restored from the browser bfcache¹, we call `switchToSavedTheme`. But `switchToSavedTheme` never looks at the `use-system-theme` preference. Further, if it can't find a saved theme, it will fall back to the default of "light".

For a user with cookies disabled² whose preferred color scheme is dark, this means the theme will wobble back and forth between dark and light. The sequence that occurs is,

1. The page is loaded. During a page load, we consult `use-system-theme`: as cookies are disabled, this preference is unset. The default is true.

   Because the default is true, we look at the preferred color scheme: for our example user, that's "dark". **The page theme is set to dark.** We'll attempt to store these preferences in localStorage, but fail due to cookies being disabled.

2. The user navigates through the docs. Subsequent page loads happen, and the same process in step 1 recurs. Previous pages are (potentially) put into the bfcache.

3. The user navigates backwards/forwards, causing a page in bfcache to be pulled out of cache. The `pageShow` event handler is triggered. However, this calls `switchToSavedTheme`: this doesn't consider the system theme, as noted above. Instead, it only looks for a saved theme. However, with cookies disabled, there is none. It defaults to light. **The page theme is set to light!** The user wonders why the dark theme is lost.

There are effectively two functions trying to determine and apply the correct theme: `updateSystemTheme` and `switchToSavedTheme`. Thus, we merge them into just one: `updateTheme`. This function contains all the logic for determining the correct theme, and is called in all circumstances where we need to set the theme:

* The initial page load
* If the browser preferred color scheme (i.e., light/dark mode) is changed
* If the page is restored from bfcache
* If the user updates the theme preferences (i.e., in `settings.js`)

Fixes rust-lang#94250.

¹bfcache: https://web.dev/bfcache/ The bfcache is used to sleep a page, if the user navigates away from it, and to restore it from cache if the user returns to it.

²Note that the browser preference that enables/disables cookies really controls many forms of storage. The same preference thus also affects localStorage. (This is so a normal browser user doesn't need to understand the distinction between "cookies" and "localStorage".)
…r=scottmcm

Make Vec::clone_from and slice::clone_into share the same code

In the past, `Vec::clone_from` was implemented using `slice::clone_into`. The code from `clone_into` was later duplicated into `clone_from` in 8725e4c, which is the commit that adds custom allocator support to Vec. Presumably this was done because the `slice::clone_into` method only works for vecs with the default allocator so it would have the wrong type to clone into `Vec<T, A>`.

Later on in 3613980 the code for the two methods diverged because the `Vec::clone_from` version gained a specialization to optimize the case when T is Copy. In order to reduce code duplication and make them both be able to take advantage of this specialization, this PR moves the specialization into the slice module and makes vec use it again.
use a more descriptive name

I found it hard to distinguish between the two method names.

Also, the comment just repeats the `expect` string.
…r_lang_attr, r=Nilstrieb

`has_allow_dead_code_or_lang_attr` micro refactor

r? `@Nilstrieb`
Change turbofish context link to an archive link

The original tweet in the chain linked to (via quote tweet), and thus the through line of links back to Anna's tweet where she named the turbofish (https://web.archive.org/web/20210911061514/https://twitter.com/whoisaldeka/status/914914008225816576) are lost as the user whoisaldeka has deleted their twitter account.

Switching to an archive link preserves this through line, allowing someone to browse back to see the point at which Anna created the turbofish, as was the original intent of including this context.

I was sharing this test with some friends as I often do, and noticed the changes (I had only seen the version from before her death previously). Looking for context myself, I realized the deleted twitter account was breaking an important link in the chain for the context of who Anna was to begin with, and the exact moment the turbofish was so named.

As an alternative to using an archive, we could link to both the tweet where Anna names the turbofish, and the tweet where she refers to herself as its guardian, as two separate links - not requiring the quote tweet to connect them.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jan 30, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Jan 30, 2023

📌 Commit d79a40d has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 30, 2023
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 30, 2023
@bors
Copy link
Contributor

bors commented Jan 30, 2023

⌛ Testing commit d79a40d with merge a322848...

@bors
Copy link
Contributor

bors commented Jan 30, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing a322848 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 30, 2023
@bors bors merged commit a322848 into rust-lang:master Jan 30, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 30, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Perf Build Sha
#107469 3144ed8c0d3908ebf9254514a5b1bd62456518a1
#107465 897ec3b868f859910dbd334ff24eddc3b6608640
#107455 56e7d1f65b9c5f1845c7b2e1396a9be85a3edac7
#107424 504fad97f62e78b851f89e1834fbb4cc614abaa6
#107177 f1a88ba11c89ff814cfa3fd1d2578cff9d99acb9
#107172 49481b63eb10f5b3a2a4f18b12a22d1164795729
#107125 5d02eb6e95658321c97d7ea5828a9a273ac47722

previous master: 006ca9b14d

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a322848): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.3% [1.3%, 1.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.7% [-0.9%, -0.4%] 7
All ❌✅ (primary) - - 0

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)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-4.7% [-5.7%, -3.6%] 2
Improvements ✅
(secondary)
-3.2% [-3.5%, -2.8%] 2
All ❌✅ (primary) -4.7% [-5.7%, -3.6%] 2

Cycles

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

@matthiaskrgr matthiaskrgr deleted the rollup-s9gn4n7 branch March 16, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants