From d369045aed63ac8b9de1ed71679fac9bb4b0340a Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 13 May 2022 10:12:32 -0700 Subject: [PATCH 1/9] Use a pointer in cell::Ref so it's not noalias --- library/core/src/cell.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 2a49017de3cc8..04bbef4844b6a 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -197,7 +197,7 @@ use crate::fmt::{self, Debug, Display}; use crate::marker::Unsize; use crate::mem; use crate::ops::{CoerceUnsized, Deref, DerefMut}; -use crate::ptr; +use crate::ptr::{self, NonNull}; /// A mutable memory location. /// @@ -896,7 +896,8 @@ impl RefCell { // SAFETY: `BorrowRef` ensures that there is only immutable access // to the value while borrowed. - Ok(Ref { value: unsafe { &*self.value.get() }, borrow: b }) + let value = unsafe { NonNull::new_unchecked(self.value.get()) }; + Ok(Ref { value, borrow: b }) } None => Err(BorrowError { // If a borrow occurred, then we must already have an outstanding borrow, @@ -1314,7 +1315,9 @@ impl Clone for BorrowRef<'_> { #[stable(feature = "rust1", since = "1.0.0")] #[must_not_suspend = "holding a Ref across suspend points can cause BorrowErrors"] pub struct Ref<'b, T: ?Sized + 'b> { - value: &'b T, + // NB: we use a pointer instead of `&'b T` to avoid `noalias` violations, because a + // `Ref` argument doesn't hold immutability for its whole scope, only until it drops. + value: NonNull, borrow: BorrowRef<'b>, } @@ -1324,7 +1327,8 @@ impl Deref for Ref<'_, T> { #[inline] fn deref(&self) -> &T { - self.value + // SAFETY: the value is accessible as long as we hold our borrow. + unsafe { self.value.as_ref() } } } @@ -1368,7 +1372,7 @@ impl<'b, T: ?Sized> Ref<'b, T> { where F: FnOnce(&T) -> &U, { - Ref { value: f(orig.value), borrow: orig.borrow } + Ref { value: NonNull::from(f(&*orig)), borrow: orig.borrow } } /// Makes a new `Ref` for an optional component of the borrowed data. The @@ -1399,8 +1403,8 @@ impl<'b, T: ?Sized> Ref<'b, T> { where F: FnOnce(&T) -> Option<&U>, { - match f(orig.value) { - Some(value) => Ok(Ref { value, borrow: orig.borrow }), + match f(&*orig) { + Some(value) => Ok(Ref { value: NonNull::from(value), borrow: orig.borrow }), None => Err(orig), } } @@ -1431,9 +1435,12 @@ impl<'b, T: ?Sized> Ref<'b, T> { where F: FnOnce(&T) -> (&U, &V), { - let (a, b) = f(orig.value); + let (a, b) = f(&*orig); let borrow = orig.borrow.clone(); - (Ref { value: a, borrow }, Ref { value: b, borrow: orig.borrow }) + ( + Ref { value: NonNull::from(a), borrow }, + Ref { value: NonNull::from(b), borrow: orig.borrow }, + ) } /// Convert into a reference to the underlying data. @@ -1467,7 +1474,8 @@ impl<'b, T: ?Sized> Ref<'b, T> { // unique reference to the borrowed RefCell. No further mutable references can be created // from the original cell. mem::forget(orig.borrow); - orig.value + // SAFETY: after forgetting, we can form a reference for the rest of lifetime `'b`. + unsafe { orig.value.as_ref() } } } From 2b8041f5746bdbd7c9f6ccf077544e1c77e927c0 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 13 May 2022 10:46:00 -0700 Subject: [PATCH 2/9] Use a pointer in cell::RefMut so it's not noalias --- library/core/src/cell.rs | 52 +++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 04bbef4844b6a..fa0206c349af6 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -194,7 +194,7 @@ use crate::cmp::Ordering; use crate::fmt::{self, Debug, Display}; -use crate::marker::Unsize; +use crate::marker::{PhantomData, Unsize}; use crate::mem; use crate::ops::{CoerceUnsized, Deref, DerefMut}; use crate::ptr::{self, NonNull}; @@ -981,8 +981,9 @@ impl RefCell { self.borrowed_at.set(Some(crate::panic::Location::caller())); } - // SAFETY: `BorrowRef` guarantees unique access. - Ok(RefMut { value: unsafe { &mut *self.value.get() }, borrow: b }) + // SAFETY: `BorrowRefMut` guarantees unique access. + let value = unsafe { NonNull::new_unchecked(self.value.get()) }; + Ok(RefMut { value, borrow: b, marker: PhantomData }) } None => Err(BorrowMutError { // If a borrow occurred, then we must already have an outstanding borrow, @@ -1515,13 +1516,13 @@ impl<'b, T: ?Sized> RefMut<'b, T> { /// ``` #[stable(feature = "cell_map", since = "1.8.0")] #[inline] - pub fn map(orig: RefMut<'b, T>, f: F) -> RefMut<'b, U> + pub fn map(mut orig: RefMut<'b, T>, f: F) -> RefMut<'b, U> where F: FnOnce(&mut T) -> &mut U, { // FIXME(nll-rfc#40): fix borrow-check - let RefMut { value, borrow } = orig; - RefMut { value: f(value), borrow } + let value = NonNull::from(f(&mut *orig)); + RefMut { value, borrow: orig.borrow, marker: PhantomData } } /// Makes a new `RefMut` for an optional component of the borrowed data. The @@ -1556,23 +1557,20 @@ impl<'b, T: ?Sized> RefMut<'b, T> { /// ``` #[unstable(feature = "cell_filter_map", reason = "recently added", issue = "81061")] #[inline] - pub fn filter_map(orig: RefMut<'b, T>, f: F) -> Result, Self> + pub fn filter_map(mut orig: RefMut<'b, T>, f: F) -> Result, Self> where F: FnOnce(&mut T) -> Option<&mut U>, { // FIXME(nll-rfc#40): fix borrow-check - let RefMut { value, borrow } = orig; - let value = value as *mut T; // SAFETY: function holds onto an exclusive reference for the duration // of its call through `orig`, and the pointer is only de-referenced // inside of the function call never allowing the exclusive reference to // escape. - match f(unsafe { &mut *value }) { - Some(value) => Ok(RefMut { value, borrow }), - None => { - // SAFETY: same as above. - Err(RefMut { value: unsafe { &mut *value }, borrow }) + match f(&mut *orig) { + Some(value) => { + Ok(RefMut { value: NonNull::from(value), borrow: orig.borrow, marker: PhantomData }) } + None => Err(orig), } } @@ -1604,15 +1602,18 @@ impl<'b, T: ?Sized> RefMut<'b, T> { #[stable(feature = "refcell_map_split", since = "1.35.0")] #[inline] pub fn map_split( - orig: RefMut<'b, T>, + mut orig: RefMut<'b, T>, f: F, ) -> (RefMut<'b, U>, RefMut<'b, V>) where F: FnOnce(&mut T) -> (&mut U, &mut V), { - let (a, b) = f(orig.value); let borrow = orig.borrow.clone(); - (RefMut { value: a, borrow }, RefMut { value: b, borrow: orig.borrow }) + let (a, b) = f(&mut *orig); + ( + RefMut { value: NonNull::from(a), borrow, marker: PhantomData }, + RefMut { value: NonNull::from(b), borrow: orig.borrow, marker: PhantomData }, + ) } /// Convert into a mutable reference to the underlying data. @@ -1638,14 +1639,15 @@ impl<'b, T: ?Sized> RefMut<'b, T> { /// assert!(cell.try_borrow_mut().is_err()); /// ``` #[unstable(feature = "cell_leak", issue = "69099")] - pub fn leak(orig: RefMut<'b, T>) -> &'b mut T { + pub fn leak(mut orig: RefMut<'b, T>) -> &'b mut T { // By forgetting this BorrowRefMut we ensure that the borrow counter in the RefCell can't // go back to UNUSED within the lifetime `'b`. Resetting the reference tracking state would // require a unique reference to the borrowed RefCell. No further references can be created // from the original cell within that lifetime, making the current borrow the only // reference for the remaining lifetime. mem::forget(orig.borrow); - orig.value + // SAFETY: after forgetting, we can form a reference for the rest of lifetime `'b`. + unsafe { orig.value.as_mut() } } } @@ -1700,8 +1702,12 @@ impl<'b> BorrowRefMut<'b> { #[stable(feature = "rust1", since = "1.0.0")] #[must_not_suspend = "holding a RefMut across suspend points can cause BorrowErrors"] pub struct RefMut<'b, T: ?Sized + 'b> { - value: &'b mut T, + // NB: we use a pointer instead of `&'b mut T` to avoid `noalias` violations, because a + // `RefMut` argument doesn't hold exclusivity for its whole scope, only until it drops. + value: NonNull, borrow: BorrowRefMut<'b>, + // NonNull is covariant over T, so we need to reintroduce invariance. + marker: PhantomData<&'b mut T>, } #[stable(feature = "rust1", since = "1.0.0")] @@ -1710,7 +1716,8 @@ impl Deref for RefMut<'_, T> { #[inline] fn deref(&self) -> &T { - self.value + // SAFETY: the value is accessible as long as we hold our borrow. + unsafe { self.value.as_ref() } } } @@ -1718,7 +1725,8 @@ impl Deref for RefMut<'_, T> { impl DerefMut for RefMut<'_, T> { #[inline] fn deref_mut(&mut self) -> &mut T { - self.value + // SAFETY: the value is accessible as long as we hold our borrow. + unsafe { self.value.as_mut() } } } From 15d8c008203208402bd234aec66b07dbe2824ec7 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 13 May 2022 11:25:51 -0700 Subject: [PATCH 3/9] Test RefCell aliasing --- src/test/codegen/noalias-refcell.rs | 14 +++++++++++ src/test/ui/issues/issue-63787.rs | 36 +++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 src/test/codegen/noalias-refcell.rs create mode 100644 src/test/ui/issues/issue-63787.rs diff --git a/src/test/codegen/noalias-refcell.rs b/src/test/codegen/noalias-refcell.rs new file mode 100644 index 0000000000000..dba73937abf17 --- /dev/null +++ b/src/test/codegen/noalias-refcell.rs @@ -0,0 +1,14 @@ +// compile-flags: -O -C no-prepopulate-passes -Z mutable-noalias=yes + +#![crate_type = "lib"] + +use std::cell::{Ref, RefCell, RefMut}; + +// Make sure that none of the arguments get a `noalias` attribute, because +// the `RefCell` might alias writes after either `Ref`/`RefMut` is dropped. + +// CHECK-LABEL: @maybe_aliased( +// CHECK-NOT: noalias +// CHECK-SAME: %_refcell +#[no_mangle] +pub unsafe fn maybe_aliased(_: Ref<'_, i32>, _: RefMut<'_, i32>, _refcell: &RefCell) {} diff --git a/src/test/ui/issues/issue-63787.rs b/src/test/ui/issues/issue-63787.rs new file mode 100644 index 0000000000000..cba079b231522 --- /dev/null +++ b/src/test/ui/issues/issue-63787.rs @@ -0,0 +1,36 @@ +// run-pass +// compile-flags: -O + +// Make sure that `Ref` and `RefMut` do not make false promises about aliasing, +// because once they drop, their reference/pointer can alias other writes. + +// Adapted from comex's proof of concept: +// https://github.com/rust-lang/rust/issues/63787#issuecomment-523588164 + +use std::cell::RefCell; +use std::ops::Deref; + +pub fn break_if_r_is_noalias(rc: &RefCell, r: impl Deref) -> i32 { + let ptr1 = &*r as *const i32; + let a = *r; + drop(r); + *rc.borrow_mut() = 2; + let r2 = rc.borrow(); + let ptr2 = &*r2 as *const i32; + if ptr2 != ptr1 { + panic!(); + } + // If LLVM knows the pointers are the same, and if `r` was `noalias`, + // then it may replace this with `a + a`, ignoring the earlier write. + a + *r2 +} + +fn main() { + let mut rc = RefCell::new(1); + let res = break_if_r_is_noalias(&rc, rc.borrow()); + assert_eq!(res, 3); + + *rc.get_mut() = 1; + let res = break_if_r_is_noalias(&rc, rc.borrow_mut()); + assert_eq!(res, 3); +} From 015e2ae76984144a7ece6d8169ff42b6dc7d6870 Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 14 May 2022 03:41:06 +0200 Subject: [PATCH 4/9] Allow the unused_macro_rules lint for now This makes the transition easier as e.g. allow directives won't fire the unknown lint warning once it is turned to warn by default in the future. This is especially important compared to other lints in the unused group because the _ prefix trick doesn't exist for macro rules, so allowing is the only option (either of unused_macro_rules, or of the entire unused group, but that is not as informative to readers). Allowing the lint also makes it possible to work on possible heuristics for disabling the macro in specific cases. --- compiler/rustc_lint_defs/src/builtin.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 7ebb1e85cdb23..f942970278399 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -790,6 +790,7 @@ declare_lint! { /// ### Example /// /// ```rust + /// #[warn(unused_macro_rules)] /// macro_rules! unused_empty { /// (hello) => { println!("Hello, world!") }; // This rule is unused /// () => { println!("empty") }; // This rule is used @@ -814,7 +815,7 @@ declare_lint! { /// /// [`macro_export` attribute]: https://doc.rust-lang.org/reference/macros-by-example.html#path-based-scope pub UNUSED_MACRO_RULES, - Warn, + Allow, "detects macro rules that were not used" } From 53b6bf496461bed9f5ae9e049dc1e137ca5363a8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 14 May 2022 13:50:52 +0200 Subject: [PATCH 5/9] Add new eslint rule about brace style --- src/librustdoc/html/static/.eslintrc.js | 5 ++ src/librustdoc/html/static/js/main.js | 4 +- .../html/static/js/scrape-examples.js | 4 +- src/librustdoc/html/static/js/search.js | 73 ++++++++++++------- 4 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/html/static/.eslintrc.js b/src/librustdoc/html/static/.eslintrc.js index 52577b228aa13..7634a15b9bd19 100644 --- a/src/librustdoc/html/static/.eslintrc.js +++ b/src/librustdoc/html/static/.eslintrc.js @@ -29,5 +29,10 @@ module.exports = { "no-var": ["error"], "prefer-const": ["error"], "prefer-arrow-callback": ["error"], + "brace-style": [ + "error", + "1tbs", + { "allowSingleLine": false } + ], } }; diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 336223ad28f32..f748616acca04 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -841,8 +841,8 @@ function loadCss(cssFileName) { onEachLazy(document.getElementsByClassName("rustdoc-toggle"), e => { if (e.parentNode.id !== "implementations-list" || (!hasClass(e, "implementors-toggle") && - !hasClass(e, "type-contents-toggle"))) - { + !hasClass(e, "type-contents-toggle")) + ) { e.open = false; } }); diff --git a/src/librustdoc/html/static/js/scrape-examples.js b/src/librustdoc/html/static/js/scrape-examples.js index 408b7e19feadd..7b9d86a851b19 100644 --- a/src/librustdoc/html/static/js/scrape-examples.js +++ b/src/librustdoc/html/static/js/scrape-examples.js @@ -98,7 +98,9 @@ // visible. This is necessary since updateScrapedExample calls scrollToLoc which // depends on offsetHeight, a property that requires an element to be visible to // compute correctly. - setTimeout(() => { onEachLazy(moreExamples, updateScrapedExample); }); + setTimeout(() => { + onEachLazy(moreExamples, updateScrapedExample); + }); }, {once: true}); }); })(); diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index b596adf32c6fd..0be70d77d06e4 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -320,8 +320,8 @@ window.initSearch = rawSearchIndex => { if (foundExclamation) { throw new Error("Cannot have more than one `!` in an ident"); } else if (parserState.pos + 1 < parserState.length && - isIdentCharacter(parserState.userQuery[parserState.pos + 1])) - { + isIdentCharacter(parserState.userQuery[parserState.pos + 1]) + ) { throw new Error("`!` can only be at the end of an ident"); } foundExclamation = true; @@ -330,12 +330,10 @@ window.initSearch = rawSearchIndex => { } else if ( isStopCharacter(c) || isSpecialStartCharacter(c) || - isSeparatorCharacter(c)) - { + isSeparatorCharacter(c) + ) { break; - } - // If we allow paths ("str::string" for example). - else if (c === ":") { + } else if (c === ":") { // If we allow paths ("str::string" for example). if (!isPathStart(parserState)) { break; } @@ -372,8 +370,8 @@ window.initSearch = rawSearchIndex => { end = getIdentEndPosition(parserState); } if (parserState.pos < parserState.length && - parserState.userQuery[parserState.pos] === "<") - { + parserState.userQuery[parserState.pos] === "<" + ) { if (isInGenerics) { throw new Error("Unexpected `<` after `<`"); } else if (start >= end) { @@ -592,8 +590,8 @@ window.initSearch = rawSearchIndex => { if (elem && elem.value !== "All crates" && - hasOwnPropertyRustdoc(rawSearchIndex, elem.value)) - { + hasOwnPropertyRustdoc(rawSearchIndex, elem.value) + ) { return elem.value; } return null; @@ -786,37 +784,51 @@ window.initSearch = rawSearchIndex => { // sort by exact match with regard to the last word (mismatch goes later) a = (aaa.word !== userQuery); b = (bbb.word !== userQuery); - if (a !== b) { return a - b; } + if (a !== b) { + return a - b; + } // Sort by non levenshtein results and then levenshtein results by the distance // (less changes required to match means higher rankings) a = (aaa.lev); b = (bbb.lev); - if (a !== b) { return a - b; } + if (a !== b) { + return a - b; + } // sort by crate (non-current crate goes later) a = (aaa.item.crate !== window.currentCrate); b = (bbb.item.crate !== window.currentCrate); - if (a !== b) { return a - b; } + if (a !== b) { + return a - b; + } // sort by item name length (longer goes later) a = aaa.word.length; b = bbb.word.length; - if (a !== b) { return a - b; } + if (a !== b) { + return a - b; + } // sort by item name (lexicographically larger goes later) a = aaa.word; b = bbb.word; - if (a !== b) { return (a > b ? +1 : -1); } + if (a !== b) { + return (a > b ? +1 : -1); + } // sort by index of keyword in item name (no literal occurrence goes later) a = (aaa.index < 0); b = (bbb.index < 0); - if (a !== b) { return a - b; } + if (a !== b) { + return a - b; + } // (later literal occurrence, if any, goes later) a = aaa.index; b = bbb.index; - if (a !== b) { return a - b; } + if (a !== b) { + return a - b; + } // special precedence for primitive and keyword pages if ((aaa.item.ty === TY_PRIMITIVE && bbb.item.ty !== TY_KEYWORD) || @@ -831,17 +843,23 @@ window.initSearch = rawSearchIndex => { // sort by description (no description goes later) a = (aaa.item.desc === ""); b = (bbb.item.desc === ""); - if (a !== b) { return a - b; } + if (a !== b) { + return a - b; + } // sort by type (later occurrence in `itemTypes` goes later) a = aaa.item.ty; b = bbb.item.ty; - if (a !== b) { return a - b; } + if (a !== b) { + return a - b; + } // sort by path (lexicographically larger goes later) a = aaa.item.path; b = bbb.item.path; - if (a !== b) { return (a > b ? +1 : -1); } + if (a !== b) { + return (a > b ? +1 : -1); + } // que sera, sera return 0; @@ -1315,16 +1333,15 @@ window.initSearch = rawSearchIndex => { } if (searchWord.indexOf(elem.pathLast) > -1 || - row.normalizedName.indexOf(elem.pathLast) > -1) - { + row.normalizedName.indexOf(elem.pathLast) > -1 + ) { // filter type: ... queries if (!results_others[fullId] !== undefined) { index = row.normalizedName.indexOf(elem.pathLast); } } lev = levenshtein(searchWord, elem.pathLast); - if (lev > 0 && elem.pathLast.length > 2 && searchWord.indexOf(elem.pathLast) > -1) - { + if (lev > 0 && elem.pathLast.length > 2 && searchWord.indexOf(elem.pathLast) > -1) { if (elem.pathLast.length < 6) { lev = 1; } else { @@ -1670,8 +1687,8 @@ window.initSearch = rawSearchIndex => { // By default, the search DOM element is "empty" (meaning it has no children not // text content). Once a search has been run, it won't be empty, even if you press // ESC or empty the search input (which also "cancels" the search). - && (!search.firstChild || search.firstChild.innerText !== searchState.loadingText))) - { + && (!search.firstChild || search.firstChild.innerText !== searchState.loadingText)) + ) { const elem = document.createElement("a"); elem.href = results.others[0].href; removeClass(elem, "active"); @@ -1766,7 +1783,7 @@ window.initSearch = rawSearchIndex => { let i = 0; for (const elem of elems) { const j = i; - elem.onclick = () => { printTab(j); }; + elem.onclick = () => printTab(j); searchState.focusedByTab.push(null); i += 1; } From 5e01ba36c9f1037c4cf3e7421413fc6c41f85d05 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 10 May 2022 16:09:41 +0200 Subject: [PATCH 6/9] Improve settings menu display --- src/librustdoc/html/static/css/rustdoc.css | 33 ++++++++ src/librustdoc/html/static/css/themes/ayu.css | 6 +- .../html/static/css/themes/dark.css | 6 +- .../html/static/css/themes/light.css | 6 +- src/librustdoc/html/static/js/settings.js | 82 +++++++++++++------ src/librustdoc/html/templates/page.html | 8 +- 6 files changed, 110 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 0f4d842f4336d..57c2ba8611443 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1404,6 +1404,15 @@ pre.rust { border-radius: 2px; cursor: pointer; } +#settings-menu { + padding: 0; +} +#settings-menu > a { + padding: 5px; + width: 100%; + height: 100%; + display: block; +} @keyframes rotating { from { @@ -1416,6 +1425,30 @@ pre.rust { #settings-menu.rotate img { animation: rotating 2s linear infinite; } +#settings-menu #settings { + position: absolute; + right: 0; + z-index: 1; + display: block; + margin-top: 7px; + border-radius: 3px; + border: 1px solid; +} +#settings-menu #settings .setting-line { + margin: 0.6em; +} +/* This rule is to draw the little arrow connecting the settings menu to the gear icon. */ +#settings-menu #settings::before { + content: ''; + position: absolute; + right: 11px; + border: solid; + border-width: 1px 1px 0 0; + display: inline-block; + padding: 4px; + transform: rotate(-45deg); + top: -5px; +} #help-button { font-family: "Fira Sans", Arial, sans-serif; diff --git a/src/librustdoc/html/static/css/themes/ayu.css b/src/librustdoc/html/static/css/themes/ayu.css index b1bf06c1865c7..ad8f642152bab 100644 --- a/src/librustdoc/html/static/css/themes/ayu.css +++ b/src/librustdoc/html/static/css/themes/ayu.css @@ -5,7 +5,7 @@ Original by Dempfi (https://github.com/dempfi/ayu) /* General structure and fonts */ -body { +body, #settings-menu #settings, #settings-menu #settings::before { background-color: #0f1419; color: #c5c5c5; } @@ -541,6 +541,10 @@ kbd { filter: invert(100); } +#settings-menu #settings, #settings-menu #settings::before { + border-color: #5c6773; +} + #copy-path { color: #fff; } diff --git a/src/librustdoc/html/static/css/themes/dark.css b/src/librustdoc/html/static/css/themes/dark.css index 236304ccc9f1b..dc38b04c88789 100644 --- a/src/librustdoc/html/static/css/themes/dark.css +++ b/src/librustdoc/html/static/css/themes/dark.css @@ -1,4 +1,4 @@ -body { +body, #settings-menu #settings, #settings-menu #settings::before { background-color: #353535; color: #ddd; } @@ -420,6 +420,10 @@ kbd { border-color: #ffb900; } +#settings-menu #settings, #settings-menu #settings::before { + border-color: #d2d2d2; +} + #copy-path { color: #999; } diff --git a/src/librustdoc/html/static/css/themes/light.css b/src/librustdoc/html/static/css/themes/light.css index c923902aba2d3..48f67af040e6d 100644 --- a/src/librustdoc/html/static/css/themes/light.css +++ b/src/librustdoc/html/static/css/themes/light.css @@ -1,6 +1,6 @@ /* General structure and fonts */ -body { +body, #settings-menu #settings, #settings-menu #settings::before { background-color: white; color: black; } @@ -405,6 +405,10 @@ kbd { border-color: #717171; } +#settings-menu #settings, #settings-menu #settings::before { + border-color: #DDDDDD; +} + #copy-path { color: #999; } diff --git a/src/librustdoc/html/static/js/settings.js b/src/librustdoc/html/static/js/settings.js index ad32a19389389..df828b5ce4c3a 100644 --- a/src/librustdoc/html/static/js/settings.js +++ b/src/librustdoc/html/static/js/settings.js @@ -1,7 +1,7 @@ // Local js definitions: /* global getSettingValue, getVirtualKey, updateLocalStorage, updateSystemTheme */ -/* global addClass, removeClass, onEach, onEachLazy, NOT_DISPLAYED_ID */ -/* global MAIN_ID, getVar, getSettingsButton, switchDisplayedElement, getNotDisplayedElem */ +/* global addClass, removeClass, onEach, onEachLazy */ +/* global MAIN_ID, getVar, getSettingsButton */ "use strict"; @@ -206,38 +206,60 @@ ]; // Then we build the DOM. - const el = document.createElement("section"); - el.id = "settings"; - let innerHTML = ` -
+ let innerHTML = ""; + let elementKind = "div"; + + if (isSettingsPage) { + elementKind = "section"; + innerHTML = `

Rustdoc settings

- `; - - if (isSettingsPage) { - innerHTML += - "Back"; - } else { - innerHTML += "Back"; + + Back + +
`; } - innerHTML += ` -
-
${buildSettingsPageSections(settings)}
`; + innerHTML += `
${buildSettingsPageSections(settings)}
`; + const el = document.createElement(elementKind); + el.id = "settings"; el.innerHTML = innerHTML; if (isSettingsPage) { document.getElementById(MAIN_ID).appendChild(el); } else { - getNotDisplayedElem().appendChild(el); + el.setAttribute("tabindex", "-1"); + getSettingsButton().appendChild(el); } return el; } const settingsMenu = buildSettingsPage(); + function displaySettings() { + settingsMenu.style.display = ""; + } + + function elemIsInParent(elem, parent) { + while (elem && elem !== document.body) { + if (elem === parent) { + return true; + } + elem = elem.parentElement; + } + return false; + } + + function blurHandler(event) { + const settingsButton = getSettingsButton(); + if (!elemIsInParent(document.activeElement, settingsButton) && + !elemIsInParent(event.relatedTarget, settingsButton)) + { + window.hideSettings(); + } + } + if (isSettingsPage) { // We replace the existing "onclick" callback to do nothing if clicked. getSettingsButton().onclick = function(event) { @@ -246,17 +268,27 @@ } else { // We replace the existing "onclick" callback. const settingsButton = getSettingsButton(); + const settingsMenu = document.getElementById("settings"); + window.hideSettings = function() { + settingsMenu.style.display = "none"; + }; settingsButton.onclick = function(event) { + if (elemIsInParent(event.target, settingsMenu)) { + return; + } event.preventDefault(); - if (settingsMenu.parentElement.id === NOT_DISPLAYED_ID) { - switchDisplayedElement(settingsMenu); - } else { + if (settingsMenu.style.display !== "none") { window.hideSettings(); + } else { + displaySettings(); } }; - window.hideSettings = function() { - switchDisplayedElement(null); - }; + settingsButton.onblur = blurHandler; + settingsButton.querySelector("a").onblur = blurHandler; + onEachLazy(settingsMenu.querySelectorAll("input"), el => { + el.onblur = blurHandler; + }); + settingsMenu.onblur = blurHandler; } // We now wait a bit for the web browser to end re-computing the DOM... @@ -264,7 +296,7 @@ setEvents(settingsMenu); // The setting menu is already displayed if we're on the settings page. if (!isSettingsPage) { - switchDisplayedElement(settingsMenu); + displaySettings(); } removeClass(getSettingsButton(), "rotate"); }, 0); diff --git a/src/librustdoc/html/templates/page.html b/src/librustdoc/html/templates/page.html index 470cce93a5020..e57d08c9456e1 100644 --- a/src/librustdoc/html/templates/page.html +++ b/src/librustdoc/html/templates/page.html @@ -126,10 +126,12 @@

placeholder="Click or press ‘S’ to search, ‘?’ for more options…" {# -#} type="search"> {#- -#} {#- -#} - {#- -#} - Change settings + {#- -#} + Change settings {#- -#} - {#- -#} + {#- -#} + {#- -#} {#- -#} {#- -#} From e8762757c3628eed11bd94db07dc0bc94542899a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 11 May 2022 23:11:18 +0200 Subject: [PATCH 7/9] Remove theme picker button --- src/librustdoc/html/markdown.rs | 2 - src/librustdoc/html/render/write_shared.rs | 1 - src/librustdoc/html/static/css/noscript.css | 4 - src/librustdoc/html/static/css/rustdoc.css | 22 +-- src/librustdoc/html/static/css/themes/ayu.css | 13 +- .../html/static/css/themes/dark.css | 11 +- .../html/static/css/themes/light.css | 11 +- src/librustdoc/html/static/js/main.js | 137 +----------------- src/librustdoc/html/static_files.rs | 3 - src/librustdoc/html/templates/page.html | 7 - 10 files changed, 12 insertions(+), 199 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 56a085c298250..5ba3bdc12eda6 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1448,8 +1448,6 @@ fn init_id_map() -> FxHashMap, usize> { // used in tera template files). map.insert("mainThemeStyle".into(), 1); map.insert("themeStyle".into(), 1); - map.insert("theme-picker".into(), 1); - map.insert("theme-choices".into(), 1); map.insert("settings-menu".into(), 1); map.insert("help-button".into(), 1); map.insert("main-content".into(), 1); diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index 68f2a54ddeb05..702bccb45cf93 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -238,7 +238,6 @@ pub(super) fn write_shared( write_toolchain("favicon-16x16.png", static_files::RUST_FAVICON_PNG_16)?; write_toolchain("favicon-32x32.png", static_files::RUST_FAVICON_PNG_32)?; } - write_toolchain("brush.svg", static_files::BRUSH_SVG)?; write_toolchain("wheel.svg", static_files::WHEEL_SVG)?; write_toolchain("clipboard.svg", static_files::CLIPBOARD_SVG)?; write_toolchain("down-arrow.svg", static_files::DOWN_ARROW_SVG)?; diff --git a/src/librustdoc/html/static/css/noscript.css b/src/librustdoc/html/static/css/noscript.css index e35358c564993..0a19a99abf054 100644 --- a/src/librustdoc/html/static/css/noscript.css +++ b/src/librustdoc/html/static/css/noscript.css @@ -18,7 +18,3 @@ rules. /* The search bar and related controls don't work without JS */ display: none; } - -#theme-picker { - display: none; -} diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 57c2ba8611443..38e67c233d698 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1379,25 +1379,15 @@ pre.rust { margin-bottom: 6px; } -.theme-picker { - position: absolute; - left: -38px; - top: 4px; -} - -.theme-picker button { - outline: none; -} - #settings-menu, #help-button { margin-left: 4px; outline: none; } -#theme-picker, #copy-path { +#copy-path { height: 34px; } -#theme-picker, #settings-menu, #help-button, #copy-path { +#settings-menu > a, #help-button, #copy-path { padding: 5px; width: 33px; border: 1px solid; @@ -1422,7 +1412,7 @@ pre.rust { transform: rotate(360deg); } } -#settings-menu.rotate img { +#settings-menu.rotate > a img { animation: rotating 2s linear infinite; } #settings-menu #settings { @@ -1871,12 +1861,6 @@ details.rustdoc-toggle[open] > summary.hideme::after { margin-left: 32px; } - /* Space is at a premium on mobile, so remove the theme-picker icon. */ - #theme-picker { - display: none; - width: 0; - } - .content { margin-left: 0px; } diff --git a/src/librustdoc/html/static/css/themes/ayu.css b/src/librustdoc/html/static/css/themes/ayu.css index ad8f642152bab..ea0cb5e072696 100644 --- a/src/librustdoc/html/static/css/themes/ayu.css +++ b/src/librustdoc/html/static/css/themes/ayu.css @@ -531,13 +531,13 @@ kbd { box-shadow: inset 0 -1px 0 #5c6773; } -#theme-picker, #settings-menu, #help-button { +#settings-menu > a, #help-button { border-color: #5c6773; background-color: #0f1419; color: #fff; } -#theme-picker > img, #settings-menu > img { +#settings-menu > a img { filter: invert(100); } @@ -555,8 +555,7 @@ kbd { filter: invert(100%); } -#theme-picker:hover, #theme-picker:focus, -#settings-menu:hover, #settings-menu:focus, +#settings-menu > a:hover, #settings-menu > a:focus, #help-button:hover, #help-button:focus { border-color: #e0e0e0; } @@ -574,12 +573,6 @@ kbd { background-color: rgba(110, 110, 110, 0.33); } -@media (max-width: 700px) { - #theme-picker { - background: #0f1419; - } -} - .search-results .result-name span.alias { color: #c5c5c5; } diff --git a/src/librustdoc/html/static/css/themes/dark.css b/src/librustdoc/html/static/css/themes/dark.css index dc38b04c88789..1525163f50281 100644 --- a/src/librustdoc/html/static/css/themes/dark.css +++ b/src/librustdoc/html/static/css/themes/dark.css @@ -408,14 +408,13 @@ kbd { box-shadow: inset 0 -1px 0 #c6cbd1; } -#theme-picker, #settings-menu, #help-button { +#settings-menu > a, #help-button { border-color: #e0e0e0; background: #f0f0f0; color: #000; } -#theme-picker:hover, #theme-picker:focus, -#settings-menu:hover, #settings-menu:focus, +#settings-menu > a:hover, #settings-menu > a:focus, #help-button:hover, #help-button:focus { border-color: #ffb900; } @@ -447,12 +446,6 @@ kbd { background-color: #4e4e4e; } -@media (max-width: 700px) { - #theme-picker { - background: #f0f0f0; - } -} - .search-results .result-name span.alias { color: #fff; } diff --git a/src/librustdoc/html/static/css/themes/light.css b/src/librustdoc/html/static/css/themes/light.css index 48f67af040e6d..d36a088d38e3f 100644 --- a/src/librustdoc/html/static/css/themes/light.css +++ b/src/librustdoc/html/static/css/themes/light.css @@ -394,13 +394,12 @@ kbd { box-shadow: inset 0 -1px 0 #c6cbd1; } -#theme-picker, #settings-menu, #help-button { +#settings-menu > a, #help-button { border-color: #e0e0e0; background-color: #fff; } -#theme-picker:hover, #theme-picker:focus, -#settings-menu:hover, #settings-menu:focus, +#settings-menu > a:hover, #settings-menu > a:focus, #help-button:hover, #help-button:focus { border-color: #717171; } @@ -432,12 +431,6 @@ kbd { background-color: #eee; } -@media (max-width: 700px) { - #theme-picker { - background: #fff; - } -} - .search-results .result-name span.alias { color: #000; } diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 336223ad28f32..1e4d58db196ad 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -1,7 +1,6 @@ // Local js definitions: /* global addClass, getSettingValue, hasClass, searchState */ /* global onEach, onEachLazy, removeClass */ -/* global switchTheme, useSystemTheme */ "use strict"; @@ -109,21 +108,11 @@ function getVirtualKey(ev) { return String.fromCharCode(c); } -const THEME_PICKER_ELEMENT_ID = "theme-picker"; -const THEMES_ELEMENT_ID = "theme-choices"; const MAIN_ID = "main-content"; const SETTINGS_BUTTON_ID = "settings-menu"; const ALTERNATIVE_DISPLAY_ID = "alternative-display"; const NOT_DISPLAYED_ID = "not-displayed"; -function getThemesElement() { - return document.getElementById(THEMES_ELEMENT_ID); -} - -function getThemePickerElement() { - return document.getElementById(THEME_PICKER_ELEMENT_ID); -} - function getSettingsButton() { return document.getElementById(SETTINGS_BUTTON_ID); } @@ -133,74 +122,10 @@ function getNakedUrl() { return window.location.href.split("?")[0].split("#")[0]; } -function showThemeButtonState() { - const themePicker = getThemePickerElement(); - const themeChoices = getThemesElement(); - - themeChoices.style.display = "block"; - themePicker.style.borderBottomRightRadius = "0"; - themePicker.style.borderBottomLeftRadius = "0"; -} - -function hideThemeButtonState() { - const themePicker = getThemePickerElement(); - const themeChoices = getThemesElement(); - - themeChoices.style.display = "none"; - themePicker.style.borderBottomRightRadius = "3px"; - themePicker.style.borderBottomLeftRadius = "3px"; -} - window.hideSettings = () => { // Does nothing by default. }; -// Set up the theme picker list. -(function () { - if (!document.location.href.startsWith("file:///")) { - return; - } - const themeChoices = getThemesElement(); - const themePicker = getThemePickerElement(); - const availableThemes = getVar("themes").split(","); - - removeClass(themeChoices.parentElement, "hidden"); - - function switchThemeButtonState() { - if (themeChoices.style.display === "block") { - hideThemeButtonState(); - } else { - showThemeButtonState(); - } - } - - function handleThemeButtonsBlur(e) { - const active = document.activeElement; - const related = e.relatedTarget; - - if (active.id !== THEME_PICKER_ELEMENT_ID && - (!active.parentNode || active.parentNode.id !== THEMES_ELEMENT_ID) && - (!related || - (related.id !== THEME_PICKER_ELEMENT_ID && - (!related.parentNode || related.parentNode.id !== THEMES_ELEMENT_ID)))) { - hideThemeButtonState(); - } - } - - themePicker.onclick = switchThemeButtonState; - themePicker.onblur = handleThemeButtonsBlur; - availableThemes.forEach(item => { - const but = document.createElement("button"); - but.textContent = item; - but.onclick = () => { - switchTheme(window.currentTheme, window.mainTheme, item, true); - useSystemTheme(false); - }; - but.onblur = handleThemeButtonsBlur; - themeChoices.appendChild(but); - }); -}()); - /** * This function inserts `newNode` after `referenceNode`. It doesn't work if `referenceNode` * doesn't have a parent node. @@ -512,7 +437,7 @@ function loadCss(cssFileName) { ev.preventDefault(); } searchState.defocus(); - hideThemeButtonState(); + window.hideSettings(); } const disableShortcuts = getSettingValue("disable-shortcuts") === "true"; @@ -522,8 +447,6 @@ function loadCss(cssFileName) { return; } - let themePicker; - if (document.activeElement.tagName === "INPUT") { switch (getVirtualKey(ev)) { case "Escape": @@ -553,64 +476,9 @@ function loadCss(cssFileName) { displayHelp(true, ev); break; - case "t": - case "T": - displayHelp(false, ev); - ev.preventDefault(); - themePicker = getThemePickerElement(); - themePicker.click(); - themePicker.focus(); - break; - default: - if (getThemePickerElement().parentNode.contains(ev.target)) { - handleThemeKeyDown(ev); - } - } - } - } - - function handleThemeKeyDown(ev) { - const active = document.activeElement; - const themes = getThemesElement(); - switch (getVirtualKey(ev)) { - case "ArrowUp": - ev.preventDefault(); - if (active.previousElementSibling && ev.target.id !== THEME_PICKER_ELEMENT_ID) { - active.previousElementSibling.focus(); - } else { - showThemeButtonState(); - themes.lastElementChild.focus(); - } - break; - case "ArrowDown": - ev.preventDefault(); - if (active.nextElementSibling && ev.target.id !== THEME_PICKER_ELEMENT_ID) { - active.nextElementSibling.focus(); - } else { - showThemeButtonState(); - themes.firstElementChild.focus(); - } - break; - case "Enter": - case "Return": - case "Space": - if (ev.target.id === THEME_PICKER_ELEMENT_ID && themes.style.display === "none") { - ev.preventDefault(); - showThemeButtonState(); - themes.firstElementChild.focus(); + break; } - break; - case "Home": - ev.preventDefault(); - themes.firstElementChild.focus(); - break; - case "End": - ev.preventDefault(); - themes.lastElementChild.focus(); - break; - // The escape key is handled in handleEscape, not here, - // so that pressing escape will close the menu even if it isn't focused } } @@ -1006,7 +874,6 @@ function loadCss(cssFileName) { const shortcuts = [ ["?", "Show this help dialog"], ["S", "Focus the search field"], - ["T", "Focus the theme picker menu"], ["↑", "Move up in search results"], ["↓", "Move down in search results"], ["← / →", "Switch result tab (when results focused)"], diff --git a/src/librustdoc/html/static_files.rs b/src/librustdoc/html/static_files.rs index bec5c083fed22..85ca8431d90da 100644 --- a/src/librustdoc/html/static_files.rs +++ b/src/librustdoc/html/static_files.rs @@ -41,9 +41,6 @@ crate static SCRAPE_EXAMPLES_JS: &str = include_str!("static/js/scrape-examples. crate static SCRAPE_EXAMPLES_HELP_MD: &str = include_str!("static/scrape-examples-help.md"); -/// The file contents of `brush.svg`, the icon used for the theme-switch button. -crate static BRUSH_SVG: &[u8] = include_bytes!("static/images/brush.svg"); - /// The file contents of `wheel.svg`, the icon used for the settings button. crate static WHEEL_SVG: &[u8] = include_bytes!("static/images/wheel.svg"); diff --git a/src/librustdoc/html/templates/page.html b/src/librustdoc/html/templates/page.html index e57d08c9456e1..cd672aadd7e93 100644 --- a/src/librustdoc/html/templates/page.html +++ b/src/librustdoc/html/templates/page.html @@ -108,13 +108,6 @@

{%- endif -%} {#- -#}