From 49166fb59102c677e0eefcccf9e71897896141b5 Mon Sep 17 00:00:00 2001 From: Rain <rain@sunshowers.io> Date: Thu, 22 Sep 2022 22:48:14 -0700 Subject: [PATCH 01/11] Change process spawning to inherit the parent's signal mask by default Previously, the signal mask is always reset when a child process is started. This breaks tools like `nohup` which expect `SIGHUP` to be blocked. With this change, the default behavior changes to inherit the signal mask. This also changes the signal disposition for `SIGPIPE` to only be changed if the `#[unix_sigpipe]` attribute isn't set. --- compiler/rustc_session/src/config/sigpipe.rs | 13 +-- library/std/src/rt.rs | 2 +- library/std/src/sys/unix/mod.rs | 38 ++++++++- .../sys/unix/process/process_common/tests.rs | 79 +++++++++++-------- .../std/src/sys/unix/process/process_unix.rs | 71 +++++++++-------- .../src/language-features/unix-sigpipe.md | 10 ++- .../unix_sigpipe/auxiliary/sigpipe-utils.rs | 6 +- 7 files changed, 141 insertions(+), 78 deletions(-) diff --git a/compiler/rustc_session/src/config/sigpipe.rs b/compiler/rustc_session/src/config/sigpipe.rs index a5c94118a47e2..53692ad7cc92b 100644 --- a/compiler/rustc_session/src/config/sigpipe.rs +++ b/compiler/rustc_session/src/config/sigpipe.rs @@ -1,5 +1,13 @@ //! NOTE: Keep these constants in sync with `library/std/src/sys/unix/mod.rs`! +/// The default value if `#[unix_sigpipe]` is not specified. This resolves +/// to `SIG_IGN` in `library/std/src/sys/unix/mod.rs`. +/// +/// Note that `SIG_IGN` has been the Rust default since 2014. See +/// <https://github.com/rust-lang/rust/issues/62569>. +#[allow(dead_code)] +pub const DEFAULT: u8 = 0; + /// Do not touch `SIGPIPE`. Use whatever the parent process uses. #[allow(dead_code)] pub const INHERIT: u8 = 1; @@ -15,8 +23,3 @@ pub const SIG_IGN: u8 = 2; /// such as `head -n 1`. #[allow(dead_code)] pub const SIG_DFL: u8 = 3; - -/// `SIG_IGN` has been the Rust default since 2014. See -/// <https://github.com/rust-lang/rust/issues/62569>. -#[allow(dead_code)] -pub const DEFAULT: u8 = SIG_IGN; diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index b8bcdbece0af3..9c2f0c1dd3eb6 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -89,7 +89,7 @@ macro_rules! rtunwrap { // `src/tools/tidy/src/pal.rs` for more info. On all other platforms, `sigpipe` // has a value, but its value is ignored. // -// Even though it is an `u8`, it only ever has 3 values. These are documented in +// Even though it is an `u8`, it only ever has 4 values. These are documented in // `compiler/rustc_session/src/config/sigpipe.rs`. #[cfg_attr(test, allow(dead_code))] unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { diff --git a/library/std/src/sys/unix/mod.rs b/library/std/src/sys/unix/mod.rs index c84e292eac152..3ae9b47d0d925 100644 --- a/library/std/src/sys/unix/mod.rs +++ b/library/std/src/sys/unix/mod.rs @@ -2,6 +2,7 @@ use crate::ffi::CStr; use crate::io::ErrorKind; +use crate::sync::atomic::{AtomicBool, Ordering}; pub use self::rand::hashmap_random_keys; @@ -163,17 +164,27 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { // See the other file for docs. NOTE: Make sure to keep them in // sync! mod sigpipe { + pub const DEFAULT: u8 = 0; pub const INHERIT: u8 = 1; pub const SIG_IGN: u8 = 2; pub const SIG_DFL: u8 = 3; } - let handler = match sigpipe { - sigpipe::INHERIT => None, - sigpipe::SIG_IGN => Some(libc::SIG_IGN), - sigpipe::SIG_DFL => Some(libc::SIG_DFL), + let (sigpipe_attr_specified, handler) = match sigpipe { + sigpipe::DEFAULT => (false, Some(libc::SIG_IGN)), + sigpipe::INHERIT => (true, None), + sigpipe::SIG_IGN => (true, Some(libc::SIG_IGN)), + sigpipe::SIG_DFL => (true, Some(libc::SIG_DFL)), _ => unreachable!(), }; + // The bootstrap compiler doesn't know about sigpipe::DEFAULT, and always passes in + // SIG_IGN. This causes some tests to fail because they expect SIGPIPE to be reset to + // default on process spawning (which doesn't happen if #[unix_sigpipe] is specified). + // Since we can't differentiate between the cases here, treat SIG_IGN as DEFAULT + // unconditionally. + if sigpipe_attr_specified && !(cfg!(bootstrap) && sigpipe == sigpipe::SIG_IGN) { + UNIX_SIGPIPE_ATTR_SPECIFIED.store(true, Ordering::Relaxed); + } if let Some(handler) = handler { rtassert!(signal(libc::SIGPIPE, handler) != libc::SIG_ERR); } @@ -181,6 +192,25 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { } } +// This is set (up to once) in reset_sigpipe. +#[cfg(not(any( + target_os = "espidf", + target_os = "emscripten", + target_os = "fuchsia", + target_os = "horizon" +)))] +static UNIX_SIGPIPE_ATTR_SPECIFIED: AtomicBool = AtomicBool::new(false); + +#[cfg(not(any( + target_os = "espidf", + target_os = "emscripten", + target_os = "fuchsia", + target_os = "horizon" +)))] +pub(crate) fn unix_sigpipe_attr_specified() -> bool { + UNIX_SIGPIPE_ATTR_SPECIFIED.load(Ordering::Relaxed) +} + // SAFETY: must be called only once during runtime cleanup. // NOTE: this is not guaranteed to run, for example when the program aborts. pub unsafe fn cleanup() { diff --git a/library/std/src/sys/unix/process/process_common/tests.rs b/library/std/src/sys/unix/process/process_common/tests.rs index d176b3401c03c..03631e4e33bf5 100644 --- a/library/std/src/sys/unix/process/process_common/tests.rs +++ b/library/std/src/sys/unix/process/process_common/tests.rs @@ -31,41 +31,54 @@ macro_rules! t { ignore )] fn test_process_mask() { - unsafe { - // Test to make sure that a signal mask does not get inherited. - let mut cmd = Command::new(OsStr::new("cat")); - - let mut set = mem::MaybeUninit::<libc::sigset_t>::uninit(); - let mut old_set = mem::MaybeUninit::<libc::sigset_t>::uninit(); - t!(cvt(sigemptyset(set.as_mut_ptr()))); - t!(cvt(sigaddset(set.as_mut_ptr(), libc::SIGINT))); - t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), old_set.as_mut_ptr()))); - - cmd.stdin(Stdio::MakePipe); - cmd.stdout(Stdio::MakePipe); - - let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null, true)); - let stdin_write = pipes.stdin.take().unwrap(); - let stdout_read = pipes.stdout.take().unwrap(); - - t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, old_set.as_ptr(), ptr::null_mut()))); - - t!(cvt(libc::kill(cat.id() as libc::pid_t, libc::SIGINT))); - // We need to wait until SIGINT is definitely delivered. The - // easiest way is to write something to cat, and try to read it - // back: if SIGINT is unmasked, it'll get delivered when cat is - // next scheduled. - let _ = stdin_write.write(b"Hello"); - drop(stdin_write); - - // Either EOF or failure (EPIPE) is okay. - let mut buf = [0; 5]; - if let Ok(ret) = stdout_read.read(&mut buf) { - assert_eq!(ret, 0); + // Test to make sure that a signal mask *does* get inherited. + fn test_inner(mut cmd: Command) { + unsafe { + let mut set = mem::MaybeUninit::<libc::sigset_t>::uninit(); + let mut old_set = mem::MaybeUninit::<libc::sigset_t>::uninit(); + t!(cvt(sigemptyset(set.as_mut_ptr()))); + t!(cvt(sigaddset(set.as_mut_ptr(), libc::SIGINT))); + t!(cvt_nz(libc::pthread_sigmask( + libc::SIG_SETMASK, + set.as_ptr(), + old_set.as_mut_ptr() + ))); + + cmd.stdin(Stdio::MakePipe); + cmd.stdout(Stdio::MakePipe); + + let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null, true)); + let stdin_write = pipes.stdin.take().unwrap(); + let stdout_read = pipes.stdout.take().unwrap(); + + t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, old_set.as_ptr(), ptr::null_mut()))); + + t!(cvt(libc::kill(cat.id() as libc::pid_t, libc::SIGINT))); + // We need to wait until SIGINT is definitely delivered. The + // easiest way is to write something to cat, and try to read it + // back: if SIGINT is unmasked, it'll get delivered when cat is + // next scheduled. + let _ = stdin_write.write(b"Hello"); + drop(stdin_write); + + // Exactly 5 bytes should be read. + let mut buf = [0; 5]; + let ret = t!(stdout_read.read(&mut buf)); + assert_eq!(ret, 5); + assert_eq!(&buf, b"Hello"); + + t!(cat.wait()); } - - t!(cat.wait()); } + + // A plain `Command::new` uses the posix_spawn path on many platforms. + let cmd = Command::new(OsStr::new("cat")); + test_inner(cmd); + + // Specifying `pre_exec` forces the fork/exec path. + let mut cmd = Command::new(OsStr::new("cat")); + unsafe { cmd.pre_exec(Box::new(|| Ok(()))) }; + test_inner(cmd); } #[test] diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 2ff8e600f7c52..946bd75d941b0 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -278,7 +278,7 @@ impl Command { stdio: ChildPipes, maybe_envp: Option<&CStringArray>, ) -> Result<!, io::Error> { - use crate::sys::{self, cvt_r}; + use crate::sys::{self, cvt_r, unix_sigpipe_attr_specified}; if let Some(fd) = stdio.stdin.fd() { cvt_r(|| libc::dup2(fd, libc::STDIN_FILENO))?; @@ -326,30 +326,26 @@ impl Command { // emscripten has no signal support. #[cfg(not(target_os = "emscripten"))] { - use crate::mem::MaybeUninit; - use crate::sys::cvt_nz; - // Reset signal handling so the child process starts in a - // standardized state. libstd ignores SIGPIPE, and signal-handling - // libraries often set a mask. Child processes inherit ignored - // signals and the signal mask from their parent, but most - // UNIX programs do not reset these things on their own, so we - // need to clean things up now to avoid confusing the program - // we're about to run. - let mut set = MaybeUninit::<libc::sigset_t>::uninit(); - cvt(sigemptyset(set.as_mut_ptr()))?; - cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), ptr::null_mut()))?; - - #[cfg(target_os = "android")] // see issue #88585 - { - let mut action: libc::sigaction = mem::zeroed(); - action.sa_sigaction = libc::SIG_DFL; - cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?; - } - #[cfg(not(target_os = "android"))] - { - let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); - if ret == libc::SIG_ERR { - return Err(io::Error::last_os_error()); + // Inherit the signal mask from the parent rather than resetting it (i.e. do not call + // pthread_sigmask). + + // If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL. + // If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility. + // + // #[unix_sigpipe] is an opportunity to change the default here. + if !unix_sigpipe_attr_specified() { + #[cfg(target_os = "android")] // see issue #88585 + { + let mut action: libc::sigaction = mem::zeroed(); + action.sa_sigaction = libc::SIG_DFL; + cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?; + } + #[cfg(not(target_os = "android"))] + { + let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); + if ret == libc::SIG_ERR { + return Err(io::Error::last_os_error()); + } } } } @@ -411,7 +407,7 @@ impl Command { envp: Option<&CStringArray>, ) -> io::Result<Option<Process>> { use crate::mem::MaybeUninit; - use crate::sys::{self, cvt_nz}; + use crate::sys::{self, cvt_nz, unix_sigpipe_attr_specified}; if self.get_gid().is_some() || self.get_uid().is_some() @@ -531,13 +527,24 @@ impl Command { cvt_nz(libc::posix_spawnattr_setpgroup(attrs.0.as_mut_ptr(), pgroup))?; } - let mut set = MaybeUninit::<libc::sigset_t>::uninit(); - cvt(sigemptyset(set.as_mut_ptr()))?; - cvt_nz(libc::posix_spawnattr_setsigmask(attrs.0.as_mut_ptr(), set.as_ptr()))?; - cvt(sigaddset(set.as_mut_ptr(), libc::SIGPIPE))?; - cvt_nz(libc::posix_spawnattr_setsigdefault(attrs.0.as_mut_ptr(), set.as_ptr()))?; + // Inherit the signal mask from this process rather than resetting it (i.e. do not call + // posix_spawnattr_setsigmask). + + // If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL. + // If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility. + // + // #[unix_sigpipe] is an opportunity to change the default here. + if !unix_sigpipe_attr_specified() { + let mut default_set = MaybeUninit::<libc::sigset_t>::uninit(); + cvt(sigemptyset(default_set.as_mut_ptr()))?; + cvt(sigaddset(default_set.as_mut_ptr(), libc::SIGPIPE))?; + cvt_nz(libc::posix_spawnattr_setsigdefault( + attrs.0.as_mut_ptr(), + default_set.as_ptr(), + ))?; + flags |= libc::POSIX_SPAWN_SETSIGDEF; + } - flags |= libc::POSIX_SPAWN_SETSIGDEF | libc::POSIX_SPAWN_SETSIGMASK; cvt_nz(libc::posix_spawnattr_setflags(attrs.0.as_mut_ptr(), flags as _))?; // Make sure we synchronize access to the global `environ` resource diff --git a/src/doc/unstable-book/src/language-features/unix-sigpipe.md b/src/doc/unstable-book/src/language-features/unix-sigpipe.md index aa39b6eb2886f..7ed6a7de895c1 100644 --- a/src/doc/unstable-book/src/language-features/unix-sigpipe.md +++ b/src/doc/unstable-book/src/language-features/unix-sigpipe.md @@ -36,7 +36,7 @@ hello world Set the `SIGPIPE` handler to `SIG_IGN` before invoking `fn main()`. This will result in `ErrorKind::BrokenPipe` errors if you program tries to write to a closed pipe. This is normally what you want if you for example write socket servers, socket clients, or pipe peers. -This is what libstd has done by default since 2014. Omitting `#[unix_sigpipe = "..."]` is the same as having `#[unix_sigpipe = "sig_ign"]`. +This is what libstd has done by default since 2014. (However, see the note on child processes below.) ### Example @@ -52,3 +52,11 @@ hello world thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1016:9 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` + +### Note on child processes + +When spawning child processes, the legacy Rust behavior if `#[unix_sigpipe]` is not specified is to +reset `SIGPIPE` to `SIG_DFL`. + +If `#[unix_sigpipe = "..."]` is specified, no matter what its value is, the signal disposition of +`SIGPIPE` is no longer reset. This means that the child inherits the parent's `SIGPIPE` behavior. diff --git a/src/test/ui/attributes/unix_sigpipe/auxiliary/sigpipe-utils.rs b/src/test/ui/attributes/unix_sigpipe/auxiliary/sigpipe-utils.rs index e8b4fe7ae5230..74fbae0350e48 100644 --- a/src/test/ui/attributes/unix_sigpipe/auxiliary/sigpipe-utils.rs +++ b/src/test/ui/attributes/unix_sigpipe/auxiliary/sigpipe-utils.rs @@ -23,9 +23,11 @@ pub fn assert_sigpipe_handler(expected_handler: SignalHandler) { SignalHandler::Ignore => libc::SIG_IGN, SignalHandler::Default => libc::SIG_DFL, }; - assert_eq!(prev, expected); + assert_eq!(prev, expected, "expected sigpipe value matches actual value"); // Unlikely to matter, but restore the old value anyway - unsafe { libc::signal(libc::SIGPIPE, prev); }; + unsafe { + libc::signal(libc::SIGPIPE, prev); + }; } } From d89fb1dee5bf47ae9bd9051b296da379e01f1755 Mon Sep 17 00:00:00 2001 From: est31 <MTest31@outlook.com> Date: Tue, 18 Oct 2022 17:10:09 +0200 Subject: [PATCH 02/11] Stabilize proc_macro::Span::source_text Splits proc_macro::Span::source_text into a new feature gate and stabilizes it. --- library/proc_macro/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/proc_macro/src/lib.rs b/library/proc_macro/src/lib.rs index f9c7d3e172cbe..8001fcff6484b 100644 --- a/library/proc_macro/src/lib.rs +++ b/library/proc_macro/src/lib.rs @@ -546,7 +546,7 @@ impl Span { /// Note: The observable result of a macro should only rely on the tokens and /// not on this source text. The result of this function is a best effort to /// be used for diagnostics only. - #[unstable(feature = "proc_macro_span", issue = "54725")] + #[stable(feature = "proc_macro_source_text", since = "CURRENT_RUSTC_VERSION")] pub fn source_text(&self) -> Option<String> { self.0.source_text() } From a01b88575e2ee7646c46fa02a94abf8cd2d33fed Mon Sep 17 00:00:00 2001 From: Michael Howell <michael@notriddle.com> Date: Tue, 18 Oct 2022 11:14:01 -0700 Subject: [PATCH 03/11] rustdoc: remove class name `location` from sidebar sibling nav This change tweaks the CSS to apply most of its styles to `.sidebar h2`, cleaning up a few redundant rules from `.mobile-topbar .location` and restoring useful navigation aids in mobile mode. --- src/librustdoc/html/render/mod.rs | 2 +- src/librustdoc/html/static/css/rustdoc.css | 43 ++++++++----------- src/librustdoc/html/static/js/main.js | 2 +- src/librustdoc/html/templates/page.html | 2 +- src/test/rustdoc-gui/mobile.goml | 2 +- src/test/rustdoc-gui/sidebar-mobile.goml | 7 ++- src/test/rustdoc-gui/sidebar.goml | 13 +++--- .../rustdoc-gui/type-declation-overflow.goml | 6 +-- 8 files changed, 39 insertions(+), 38 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index cd56d73e7d47b..eeec6f8fee778 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1884,7 +1884,7 @@ fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) { if !it.is_mod() { let path: String = cx.current.iter().map(|s| s.as_str()).intersperse("::").collect(); - write!(buffer, "<h2 class=\"location\"><a href=\"index.html\">In {}</a></h2>", path); + write!(buffer, "<h2><a href=\"index.html\">In {}</a></h2>", path); } // Closes sidebar-elems div. diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 5788363036acd..0dd77ff8d9d38 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -171,7 +171,7 @@ h1.fqn { Rustdoc-generated h2 section headings (e.g. "Implementations", "Required Methods", etc) Underlines elsewhere in the documentation break up visual flow and tend to invert section hierarchies. */ -h2, +.content h2, .top-doc .docblock > h3, .top-doc .docblock > h4 { border-bottom: 1px solid var(--headings-border-bottom-color); @@ -397,15 +397,6 @@ img { left: 0; } -.sidebar-elems, -.sidebar > .location { - padding-left: 24px; -} - -.sidebar .location { - overflow-wrap: anywhere; -} - .rustdoc.source .sidebar { width: 50px; min-width: 0px; @@ -504,8 +495,8 @@ ul.block, .block li { } .block a, -.sidebar h3 a, -h2.location a { +.sidebar h2 a, +.sidebar h3 a { display: block; padding: 0.25rem; margin-left: -0.25rem; @@ -515,8 +506,7 @@ h2.location a { } .sidebar h2 { - border-bottom: none; - font-weight: 500; + overflow-wrap: anywhere; padding: 0; margin: 0; margin-top: 0.7rem; @@ -525,11 +515,15 @@ h2.location a { .sidebar h3 { font-size: 1.125rem; /* 18px */ - font-weight: 500; padding: 0; margin: 0; } +.sidebar-elems, +.sidebar > h2 { + padding-left: 24px; +} + .sidebar a, .sidebar .current { color: var(--sidebar-link-color); } @@ -1798,18 +1792,10 @@ in storage.js plus the media query with (min-width: 701px) width: 0; } - .mobile-topbar .location a { - padding: 0; - margin: 0; - } - - .mobile-topbar .location { - border: none; - padding: 0; + .mobile-topbar h2 { + padding-bottom: 0; margin: auto 0.5em auto auto; - text-overflow: ellipsis; overflow: hidden; - white-space: nowrap; /* Rare exception to specifying font sizes in rem. Since the topbar height is specified in pixels, this also has to be specified in pixels to avoid overflowing the topbar when the user sets a bigger @@ -1817,6 +1803,13 @@ in storage.js plus the media query with (min-width: 701px) font-size: 24px; } + .mobile-topbar h2 a { + display: block; + text-overflow: ellipsis; + overflow: hidden; + white-space: nowrap; + } + .mobile-topbar .logo-container { max-height: 45px; } diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 3bcadcda534dc..b9a99e1e79919 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -55,7 +55,7 @@ function blurHandler(event, parentElem, hideCallback) { function setMobileTopbar() { // FIXME: It would be nicer to generate this text content directly in HTML, // but with the current code it's hard to get the right information in the right place. - const mobileLocationTitle = document.querySelector(".mobile-topbar h2.location"); + const mobileLocationTitle = document.querySelector(".mobile-topbar h2"); const locationTitle = document.querySelector(".sidebar h2.location"); if (mobileLocationTitle && locationTitle) { mobileLocationTitle.innerHTML = locationTitle.innerHTML; diff --git a/src/librustdoc/html/templates/page.html b/src/librustdoc/html/templates/page.html index 123bd576d6463..20a314a1c00e3 100644 --- a/src/librustdoc/html/templates/page.html +++ b/src/librustdoc/html/templates/page.html @@ -85,7 +85,7 @@ {%- endif -%} </div> {#- -#} </a> {#- -#} - <h2 class="location"></h2> {#- -#} + <h2></h2> {#- -#} </nav> {#- -#} {%- endif -%} <nav class="sidebar"> {#- -#} diff --git a/src/test/rustdoc-gui/mobile.goml b/src/test/rustdoc-gui/mobile.goml index 22a53dea616b0..704542a39d21b 100644 --- a/src/test/rustdoc-gui/mobile.goml +++ b/src/test/rustdoc-gui/mobile.goml @@ -12,7 +12,7 @@ assert-css: (".main-heading", { "flex-direction": "column" }) -assert-property: (".mobile-topbar h2.location", {"offsetHeight": 36}) +assert-property: (".mobile-topbar h2", {"offsetHeight": 36}) // Note: We can't use assert-text here because the 'Since' is set by CSS and // is therefore not part of the DOM. diff --git a/src/test/rustdoc-gui/sidebar-mobile.goml b/src/test/rustdoc-gui/sidebar-mobile.goml index 4b3ec79273d40..453873f1b81b6 100644 --- a/src/test/rustdoc-gui/sidebar-mobile.goml +++ b/src/test/rustdoc-gui/sidebar-mobile.goml @@ -23,6 +23,11 @@ assert-css: (".sidebar", {"display": "block", "left": "-1000px"}) click: ".sidebar-menu-toggle" assert-css: (".sidebar", {"left": "0px"}) +// Make sure the "struct Foo" header is hidden, since the mobile topbar already does it. +assert-css: ("//nav[contains(@class, 'sidebar')]//h2/a[text()='Foo']/parent::h2", {"display": "none"}) +// Make sure the global navigation is still here. +assert-css: ("//nav[contains(@class, 'sidebar')]//h2/a[text()='In test_docs']/parent::h2", {"display": "block"}) + // Click elsewhere. click: "body" assert-css: (".sidebar", {"display": "block", "left": "-1000px"}) @@ -39,7 +44,7 @@ assert-position: ("#method\.must_use", {"y": 45}) // Check that the bottom-most item on the sidebar menu can be scrolled fully into view. click: ".sidebar-menu-toggle" scroll-to: ".block.keyword li:nth-child(1)" -compare-elements-position-near: (".block.keyword li:nth-child(1)", ".mobile-topbar", {"y": 543}) +compare-elements-position-near: (".block.keyword li:nth-child(1)", ".mobile-topbar", {"y": 543.19}) // Now checking the background color of the sidebar. show-text: true diff --git a/src/test/rustdoc-gui/sidebar.goml b/src/test/rustdoc-gui/sidebar.goml index 54193234af9dd..c92561b0de14f 100644 --- a/src/test/rustdoc-gui/sidebar.goml +++ b/src/test/rustdoc-gui/sidebar.goml @@ -9,6 +9,7 @@ reload: assert-text: (".sidebar > .location", "Crate test_docs") // In modules, we only have one "location" element. assert-count: (".sidebar .location", 1) +assert-count: (".sidebar h2", 1) assert-text: ("#all-types", "All Items") assert-css: ("#all-types", {"color": "rgb(53, 109, 164)"}) // We check that we have the crates list and that the "current" on is "test_docs". @@ -28,7 +29,8 @@ assert-text: ("#structs + .item-table .item-left > a", "Foo") click: "#structs + .item-table .item-left > a" // PAGE: struct.Foo.html -assert-count: (".sidebar .location", 2) +assert-count: (".sidebar .location", 1) +assert-count: (".sidebar h2", 2) // We check that there is no crate listed outside of the top level. assert-false: ".sidebar-elems > .crate" @@ -60,10 +62,11 @@ assert-text: ("#functions + .item-table .item-left > a", "foobar") click: "#functions + .item-table .item-left > a" // PAGE: fn.foobar.html -// In items containing no items (like functions or constants) and in modules, we have one -// "location" elements. -assert-count: (".sidebar .location", 1) -assert-text: (".sidebar .sidebar-elems .location", "In lib2") +// In items containing no items (like functions or constants) and in modules, we have no +// "location" elements. Only the parent module h2. +assert-count: (".sidebar .location", 0) +assert-count: (".sidebar h2", 1) +assert-text: (".sidebar .sidebar-elems h2", "In lib2") // We check that we don't have the crate list. assert-false: ".sidebar-elems > .crate" diff --git a/src/test/rustdoc-gui/type-declation-overflow.goml b/src/test/rustdoc-gui/type-declation-overflow.goml index 657c3cfa4b616..fce3002e7508f 100644 --- a/src/test/rustdoc-gui/type-declation-overflow.goml +++ b/src/test/rustdoc-gui/type-declation-overflow.goml @@ -32,6 +32,6 @@ assert-property: (".item-decl pre", {"scrollWidth": "950"}) size: (600, 600) goto: "file://" + |DOC_PATH| + "/lib2/too_long/struct.SuperIncrediblyLongLongLongLongLongLongLongGigaGigaGigaMegaLongLongLongStructName.html" // It shouldn't have an overflow in the topbar either. -store-property: (scrollWidth, ".mobile-topbar .location", "scrollWidth") -assert-property: (".mobile-topbar .location", {"clientWidth": |scrollWidth|}) -assert-css: (".mobile-topbar .location", {"overflow-x": "hidden"}) +store-property: (scrollWidth, ".mobile-topbar h2", "scrollWidth") +assert-property: (".mobile-topbar h2", {"clientWidth": |scrollWidth|}) +assert-css: (".mobile-topbar h2", {"overflow-x": "hidden"}) From d96b68fc0d6f818ec606639eba543fe6eea9932f Mon Sep 17 00:00:00 2001 From: Xiretza <xiretza@xiretza.xyz> Date: Mon, 17 Oct 2022 19:41:49 +0200 Subject: [PATCH 04/11] Allow specifying multiple alternative suggestions This allows porting uses of span_suggestions() to diagnostic structs. Doesn't work for multipart_suggestions() because the rank would be reversed - the struct would specify multiple spans, each of which has multiple possible replacements, while multipart_suggestions() creates multiple possible replacements, each with multiple spans. --- compiler/rustc_errors/src/diagnostic.rs | 26 ++++- .../src/diagnostics/diagnostic_builder.rs | 2 +- .../src/diagnostics/subdiagnostic.rs | 21 ++-- .../rustc_macros/src/diagnostics/utils.rs | 105 ++++++++++++++++-- .../session-diagnostic/diagnostic-derive.rs | 38 +++++++ .../diagnostic-derive.stderr | 20 +++- .../subdiagnostic-derive.rs | 45 ++++++++ .../subdiagnostic-derive.stderr | 32 +++++- 8 files changed, 263 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 518c59dba5366..d342359ee294f 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -681,6 +681,24 @@ impl Diagnostic { msg: impl Into<SubdiagnosticMessage>, suggestions: impl Iterator<Item = String>, applicability: Applicability, + ) -> &mut Self { + self.span_suggestions_with_style( + sp, + msg, + suggestions, + applicability, + SuggestionStyle::ShowCode, + ) + } + + /// [`Diagnostic::span_suggestions()`] but you can set the [`SuggestionStyle`]. + pub fn span_suggestions_with_style( + &mut self, + sp: Span, + msg: impl Into<SubdiagnosticMessage>, + suggestions: impl Iterator<Item = String>, + applicability: Applicability, + style: SuggestionStyle, ) -> &mut Self { let mut suggestions: Vec<_> = suggestions.collect(); suggestions.sort(); @@ -691,14 +709,15 @@ impl Diagnostic { self.push_suggestion(CodeSuggestion { substitutions, msg: self.subdiagnostic_message_to_diagnostic_message(msg), - style: SuggestionStyle::ShowCode, + style, applicability, }); self } - /// Prints out a message with multiple suggested edits of the code. - /// See also [`Diagnostic::span_suggestion()`]. + /// Prints out a message with multiple suggested edits of the code, where each edit consists of + /// multiple parts. + /// See also [`Diagnostic::multipart_suggestion()`]. pub fn multipart_suggestions( &mut self, msg: impl Into<SubdiagnosticMessage>, @@ -720,6 +739,7 @@ impl Diagnostic { }); self } + /// Prints out a message with a suggested edit of the code. If the suggestion is presented /// inline, it will only show the message and not the suggestion. /// diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index dcbe89251cb36..318675ba08fd8 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -433,7 +433,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { self.formatting_init.extend(code_init); Ok(quote! { - #diag.span_suggestion_with_style( + #diag.span_suggestions_with_style( #span_field, rustc_errors::fluent::#slug, #code_field, diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index 3d4c3ab9fd7c9..ffeed99974bff 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -11,9 +11,11 @@ use crate::diagnostics::utils::{ }; use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use syn::{spanned::Spanned, Attribute, Meta, MetaList, MetaNameValue, NestedMeta, Path}; +use syn::{spanned::Spanned, Attribute, Meta, MetaList, NestedMeta, Path}; use synstructure::{BindingInfo, Structure, VariantInfo}; +use super::utils::{build_suggestion_code, AllowMultipleAlternatives}; + /// The central struct for constructing the `add_to_diagnostic` method from an annotated struct. pub(crate) struct SubdiagnosticDeriveBuilder { diag: syn::Ident, @@ -393,15 +395,16 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { let nested_name = meta.path().segments.last().unwrap().ident.to_string(); let nested_name = nested_name.as_str(); - let Meta::NameValue(MetaNameValue { lit: syn::Lit::Str(value), .. }) = meta else { - throw_invalid_nested_attr!(attr, &nested_attr); - }; - match nested_name { "code" => { - let formatted_str = self.build_format(&value.value(), value.span()); let code_field = new_code_ident(); - code.set_once((code_field, formatted_str), span); + let formatting_init = build_suggestion_code( + &code_field, + meta, + self, + AllowMultipleAlternatives::No, + ); + code.set_once((code_field, formatting_init), span); } _ => throw_invalid_nested_attr!(attr, &nested_attr, |diag| { diag.help("`code` is the only valid nested attribute") @@ -409,14 +412,14 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { } } - let Some((code_field, formatted_str)) = code.value() else { + let Some((code_field, formatting_init)) = code.value() else { span_err(span, "`#[suggestion_part(...)]` attribute without `code = \"...\"`") .emit(); return Ok(quote! {}); }; let binding = info.binding; - self.formatting_init.extend(quote! { let #code_field = #formatted_str; }); + self.formatting_init.extend(formatting_init); let code_field = if clone_suggestion_code { quote! { #code_field.clone() } } else { diff --git a/compiler/rustc_macros/src/diagnostics/utils.rs b/compiler/rustc_macros/src/diagnostics/utils.rs index 4fd4adc511267..44f65998ebaa4 100644 --- a/compiler/rustc_macros/src/diagnostics/utils.rs +++ b/compiler/rustc_macros/src/diagnostics/utils.rs @@ -2,7 +2,7 @@ use crate::diagnostics::error::{ span_err, throw_invalid_attr, throw_invalid_nested_attr, throw_span_err, DiagnosticDeriveError, }; use proc_macro::Span; -use proc_macro2::TokenStream; +use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote, ToTokens}; use std::cell::RefCell; use std::collections::{BTreeSet, HashMap}; @@ -395,6 +395,82 @@ pub(super) fn build_field_mapping<'v>(variant: &VariantInfo<'v>) -> HashMap<Stri fields_map } +#[derive(Copy, Clone, Debug)] +pub(super) enum AllowMultipleAlternatives { + No, + Yes, +} + +/// Constructs the `format!()` invocation(s) necessary for a `#[suggestion*(code = "foo")]` or +/// `#[suggestion*(code("foo", "bar"))]` attribute field +pub(super) fn build_suggestion_code( + code_field: &Ident, + meta: &Meta, + fields: &impl HasFieldMap, + allow_multiple: AllowMultipleAlternatives, +) -> TokenStream { + let values = match meta { + // `code = "foo"` + Meta::NameValue(MetaNameValue { lit: syn::Lit::Str(s), .. }) => vec![s], + // `code("foo", "bar")` + Meta::List(MetaList { nested, .. }) => { + if let AllowMultipleAlternatives::No = allow_multiple { + span_err( + meta.span().unwrap(), + "expected exactly one string literal for `code = ...`", + ) + .emit(); + vec![] + } else if nested.is_empty() { + span_err( + meta.span().unwrap(), + "expected at least one string literal for `code(...)`", + ) + .emit(); + vec![] + } else { + nested + .into_iter() + .filter_map(|item| { + if let NestedMeta::Lit(syn::Lit::Str(s)) = item { + Some(s) + } else { + span_err( + item.span().unwrap(), + "`code(...)` must contain only string literals", + ) + .emit(); + None + } + }) + .collect() + } + } + _ => { + span_err( + meta.span().unwrap(), + r#"`code = "..."`/`code(...)` must contain only string literals"#, + ) + .emit(); + vec![] + } + }; + + if let AllowMultipleAlternatives::Yes = allow_multiple { + let formatted_strings: Vec<_> = values + .into_iter() + .map(|value| fields.build_format(&value.value(), value.span())) + .collect(); + quote! { let #code_field = [#(#formatted_strings),*].into_iter(); } + } else if let [value] = values.as_slice() { + let formatted_str = fields.build_format(&value.value(), value.span()); + quote! { let #code_field = #formatted_str; } + } else { + // error handled previously + quote! { let #code_field = String::new(); } + } +} + /// Possible styles for suggestion subdiagnostics. #[derive(Clone, Copy)] pub(super) enum SuggestionKind { @@ -564,21 +640,23 @@ impl SubdiagnosticKind { let nested_name = meta.path().segments.last().unwrap().ident.to_string(); let nested_name = nested_name.as_str(); - let value = match meta { - Meta::NameValue(MetaNameValue { lit: syn::Lit::Str(value), .. }) => value, + let string_value = match meta { + Meta::NameValue(MetaNameValue { lit: syn::Lit::Str(value), .. }) => Some(value), + Meta::Path(_) => throw_invalid_nested_attr!(attr, &nested_attr, |diag| { diag.help("a diagnostic slug must be the first argument to the attribute") }), - _ => { - invalid_nested_attr(attr, &nested_attr).emit(); - continue; - } + _ => None, }; match (nested_name, &mut kind) { ("code", SubdiagnosticKind::Suggestion { code_field, .. }) => { - let formatted_str = fields.build_format(&value.value(), value.span()); - let code_init = quote! { let #code_field = #formatted_str; }; + let code_init = build_suggestion_code( + code_field, + meta, + fields, + AllowMultipleAlternatives::Yes, + ); code.set_once(code_init, span); } ( @@ -586,6 +664,11 @@ impl SubdiagnosticKind { SubdiagnosticKind::Suggestion { ref mut applicability, .. } | SubdiagnosticKind::MultipartSuggestion { ref mut applicability, .. }, ) => { + let Some(value) = string_value else { + invalid_nested_attr(attr, &nested_attr).emit(); + continue; + }; + let value = Applicability::from_str(&value.value()).unwrap_or_else(|()| { span_err(span, "invalid applicability").emit(); Applicability::Unspecified @@ -616,7 +699,7 @@ impl SubdiagnosticKind { init } else { span_err(span, "suggestion without `code = \"...\"`").emit(); - quote! { let #code_field: String = unreachable!(); } + quote! { let #code_field = std::iter::empty(); } }; } SubdiagnosticKind::Label @@ -637,7 +720,7 @@ impl quote::IdentFragment for SubdiagnosticKind { SubdiagnosticKind::Note => write!(f, "note"), SubdiagnosticKind::Help => write!(f, "help"), SubdiagnosticKind::Warn => write!(f, "warn"), - SubdiagnosticKind::Suggestion { .. } => write!(f, "suggestion_with_style"), + SubdiagnosticKind::Suggestion { .. } => write!(f, "suggestions_with_style"), SubdiagnosticKind::MultipartSuggestion { .. } => { write!(f, "multipart_suggestion_with_style") } diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs index e873c36e0b39a..8792528b17536 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs @@ -749,3 +749,41 @@ struct SubdiagnosticEagerSuggestion { #[subdiagnostic(eager)] sub: SubdiagnosticWithSuggestion, } + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SuggestionsGood { + #[suggestion(code("foo", "bar"))] + sub: Span, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SuggestionsSingleItem { + #[suggestion(code("foo"))] + sub: Span, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SuggestionsNoItem { + #[suggestion(code())] + //~^ ERROR expected at least one string literal for `code(...)` + sub: Span, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SuggestionsInvalidItem { + #[suggestion(code(foo))] + //~^ ERROR `code(...)` must contain only string literals + sub: Span, +} + +#[derive(Diagnostic)] +#[diag(compiletest::example)] +struct SuggestionsInvalidLiteral { + #[suggestion(code = 3)] + //~^ ERROR `code = "..."`/`code(...)` must contain only string literals + sub: Span, +} diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr index 7a42d618707ad..5113c8f65dc80 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr @@ -573,6 +573,24 @@ LL | #[subdiagnostic(eager)] | = help: eager subdiagnostics are not supported on lints +error: expected at least one string literal for `code(...)` + --> $DIR/diagnostic-derive.rs:770:18 + | +LL | #[suggestion(code())] + | ^^^^^^ + +error: `code(...)` must contain only string literals + --> $DIR/diagnostic-derive.rs:778:23 + | +LL | #[suggestion(code(foo))] + | ^^^ + +error: `code = "..."`/`code(...)` must contain only string literals + --> $DIR/diagnostic-derive.rs:786:18 + | +LL | #[suggestion(code = 3)] + | ^^^^^^^^ + error: cannot find attribute `nonsense` in this scope --> $DIR/diagnostic-derive.rs:55:3 | @@ -647,7 +665,7 @@ LL | arg: impl IntoDiagnosticArg, | ^^^^^^^^^^^^^^^^^ required by this bound in `DiagnosticBuilder::<'a, G>::set_arg` = note: this error originates in the derive macro `Diagnostic` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 80 previous errors +error: aborting due to 83 previous errors Some errors have detailed explanations: E0277, E0425. For more information about an error, try `rustc --explain E0277`. diff --git a/src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs b/src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs index 84ee5af42dea5..589a3babbb3ba 100644 --- a/src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs +++ b/src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs @@ -641,3 +641,48 @@ struct BJ { span: Span, r#type: String, } + +#[derive(Subdiagnostic)] +#[multipart_suggestion(parser::add_paren)] +struct BK { + #[suggestion_part(code("foo"))] + //~^ ERROR expected exactly one string literal for `code = ...` + span: Span, + r#type: String, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(parser::add_paren)] +struct BL { + #[suggestion_part(code("foo", "bar"))] + //~^ ERROR expected exactly one string literal for `code = ...` + span: Span, + r#type: String, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(parser::add_paren)] +struct BM { + #[suggestion_part(code(3))] + //~^ ERROR expected exactly one string literal for `code = ...` + span: Span, + r#type: String, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(parser::add_paren)] +struct BN { + #[suggestion_part(code())] + //~^ ERROR expected exactly one string literal for `code = ...` + span: Span, + r#type: String, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(parser::add_paren)] +struct BO { + #[suggestion_part(code = 3)] + //~^ ERROR `code = "..."`/`code(...)` must contain only string literals + span: Span, + r#type: String, +} diff --git a/src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.stderr b/src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.stderr index 171b89e657d81..16352aaf3b200 100644 --- a/src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.stderr +++ b/src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.stderr @@ -421,6 +421,36 @@ error: `#[applicability]` has no effect if all `#[suggestion]`/`#[multipart_sugg LL | #[applicability] | ^^^^^^^^^^^^^^^^ +error: expected exactly one string literal for `code = ...` + --> $DIR/subdiagnostic-derive.rs:648:23 + | +LL | #[suggestion_part(code("foo"))] + | ^^^^^^^^^^^ + +error: expected exactly one string literal for `code = ...` + --> $DIR/subdiagnostic-derive.rs:657:23 + | +LL | #[suggestion_part(code("foo", "bar"))] + | ^^^^^^^^^^^^^^^^^^ + +error: expected exactly one string literal for `code = ...` + --> $DIR/subdiagnostic-derive.rs:666:23 + | +LL | #[suggestion_part(code(3))] + | ^^^^^^^ + +error: expected exactly one string literal for `code = ...` + --> $DIR/subdiagnostic-derive.rs:675:23 + | +LL | #[suggestion_part(code())] + | ^^^^^^ + +error: `code = "..."`/`code(...)` must contain only string literals + --> $DIR/subdiagnostic-derive.rs:684:23 + | +LL | #[suggestion_part(code = 3)] + | ^^^^^^^^ + error: cannot find attribute `foo` in this scope --> $DIR/subdiagnostic-derive.rs:63:3 | @@ -481,6 +511,6 @@ error[E0425]: cannot find value `slug` in module `rustc_errors::fluent` LL | #[label(slug)] | ^^^^ not found in `rustc_errors::fluent` -error: aborting due to 68 previous errors +error: aborting due to 73 previous errors For more information about this error, try `rustc --explain E0425`. From ffe24e5bc6dbf51e8be2089b489fe31080745665 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez <guillaume.gomez@huawei.com> Date: Wed, 19 Oct 2022 11:08:13 +0200 Subject: [PATCH 05/11] Update browser-ui-test version to 0.12.7 --- .../docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version index 0ec9201397fe4..cc96715b28572 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version @@ -1 +1 @@ -0.12.6 \ No newline at end of file +0.12.7 \ No newline at end of file From afccb65c9cd1c73bc33305d93c970d2db448d162 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez <guillaume.gomez@huawei.com> Date: Wed, 19 Oct 2022 12:01:59 +0200 Subject: [PATCH 06/11] Clean up codeblock-tooltip rustdoc-gui test --- src/test/rustdoc-gui/codeblock-tooltip.goml | 168 +++++++++----------- 1 file changed, 76 insertions(+), 92 deletions(-) diff --git a/src/test/rustdoc-gui/codeblock-tooltip.goml b/src/test/rustdoc-gui/codeblock-tooltip.goml index f01e0c3c6cc74..8e681a2a0c3a3 100644 --- a/src/test/rustdoc-gui/codeblock-tooltip.goml +++ b/src/test/rustdoc-gui/codeblock-tooltip.goml @@ -2,95 +2,79 @@ goto: "file://" + |DOC_PATH| + "/test_docs/fn.foo.html" show-text: true -// Dark theme. -local-storage: {"rustdoc-theme": "dark", "rustdoc-use-system-theme": "false"} -reload: - -// compile_fail block -assert-css: (".docblock .example-wrap.compile_fail .tooltip", {"color": "rgba(255, 0, 0, 0.5)"}) -assert-css: (".docblock .example-wrap.compile_fail", {"border-left": "2px solid rgba(255, 0, 0, 0.5)"}) - -move-cursor-to: ".docblock .example-wrap.compile_fail" - -assert-css: (".docblock .example-wrap.compile_fail .tooltip", {"color": "rgb(255, 0, 0)"}) -assert-css: (".docblock .example-wrap.compile_fail", {"border-left": "2px solid rgb(255, 0, 0)"}) - -// should_panic block -assert-css: (".docblock .example-wrap.should_panic .tooltip", {"color": "rgba(255, 0, 0, 0.5)"}) -assert-css: (".docblock .example-wrap.should_panic", {"border-left": "2px solid rgba(255, 0, 0, 0.5)"}) - -move-cursor-to: ".docblock .example-wrap.should_panic" - -assert-css: (".docblock .example-wrap.should_panic .tooltip", {"color": "rgb(255, 0, 0)"}) -assert-css: (".docblock .example-wrap.should_panic", {"border-left": "2px solid rgb(255, 0, 0)"}) - -// ignore block -assert-css: (".docblock .example-wrap.ignore .tooltip", {"color": "rgba(255, 142, 0, 0.6)"}) -assert-css: (".docblock .example-wrap.ignore", {"border-left": "2px solid rgba(255, 142, 0, 0.6)"}) - -move-cursor-to: ".docblock .example-wrap.ignore" - -assert-css: (".docblock .example-wrap.ignore .tooltip", {"color": "rgb(255, 142, 0)"}) -assert-css: (".docblock .example-wrap.ignore", {"border-left": "2px solid rgb(255, 142, 0)"}) - - -// Light theme. -local-storage: {"rustdoc-theme": "light"} -reload: - -assert-css: (".docblock .example-wrap.compile_fail .tooltip", {"color": "rgba(255, 0, 0, 0.5)"}) -assert-css: (".docblock .example-wrap.compile_fail", {"border-left": "2px solid rgba(255, 0, 0, 0.5)"}) - -move-cursor-to: ".docblock .example-wrap.compile_fail" - -assert-css: (".docblock .example-wrap.compile_fail .tooltip", {"color": "rgb(255, 0, 0)"}) -assert-css: (".docblock .example-wrap.compile_fail", {"border-left": "2px solid rgb(255, 0, 0)"}) - -// should_panic block -assert-css: (".docblock .example-wrap.should_panic .tooltip", {"color": "rgba(255, 0, 0, 0.5)"}) -assert-css: (".docblock .example-wrap.should_panic", {"border-left": "2px solid rgba(255, 0, 0, 0.5)"}) - -move-cursor-to: ".docblock .example-wrap.should_panic" - -assert-css: (".docblock .example-wrap.should_panic .tooltip", {"color": "rgb(255, 0, 0)"}) -assert-css: (".docblock .example-wrap.should_panic", {"border-left": "2px solid rgb(255, 0, 0)"}) - -// ignore block -assert-css: (".docblock .example-wrap.ignore .tooltip", {"color": "rgba(255, 142, 0, 0.6)"}) -assert-css: (".docblock .example-wrap.ignore", {"border-left": "2px solid rgba(255, 142, 0, 0.6)"}) - -move-cursor-to: ".docblock .example-wrap.ignore" - -assert-css: (".docblock .example-wrap.ignore .tooltip", {"color": "rgb(255, 142, 0)"}) -assert-css: (".docblock .example-wrap.ignore", {"border-left": "2px solid rgb(255, 142, 0)"}) - - -// Ayu theme. -local-storage: {"rustdoc-theme": "ayu"} -reload: - -assert-css: (".docblock .example-wrap.compile_fail .tooltip", {"color": "rgba(255, 0, 0, 0.5)"}) -assert-css: (".docblock .example-wrap.compile_fail", {"border-left": "2px solid rgba(255, 0, 0, 0.5)"}) - -move-cursor-to: ".docblock .example-wrap.compile_fail" - -assert-css: (".docblock .example-wrap.compile_fail .tooltip", {"color": "rgb(255, 0, 0)"}) -assert-css: (".docblock .example-wrap.compile_fail", {"border-left": "2px solid rgb(255, 0, 0)"}) - -// should_panic block -assert-css: (".docblock .example-wrap.should_panic .tooltip", {"color": "rgba(255, 0, 0, 0.5)"}) -assert-css: (".docblock .example-wrap.should_panic", {"border-left": "2px solid rgba(255, 0, 0, 0.5)"}) - -move-cursor-to: ".docblock .example-wrap.should_panic" - -assert-css: (".docblock .example-wrap.should_panic .tooltip", {"color": "rgb(255, 0, 0)"}) -assert-css: (".docblock .example-wrap.should_panic", {"border-left": "2px solid rgb(255, 0, 0)"}) - -// ignore block -assert-css: (".docblock .example-wrap.ignore .tooltip", {"color": "rgba(255, 142, 0, 0.6)"}) -assert-css: (".docblock .example-wrap.ignore", {"border-left": "2px solid rgba(255, 142, 0, 0.6)"}) - -move-cursor-to: ".docblock .example-wrap.ignore" - -assert-css: (".docblock .example-wrap.ignore .tooltip", {"color": "rgb(255, 142, 0)"}) -assert-css: (".docblock .example-wrap.ignore", {"border-left": "2px solid rgb(255, 142, 0)"}) +define-function: ( + "check-colors", + (theme), + [ + // Setting the theme. + ("local-storage", {"rustdoc-theme": |theme|, "rustdoc-use-system-theme": "false"}), + ("reload"), + + // compile_fail block + ("assert-css", ( + ".docblock .example-wrap.compile_fail .tooltip", + {"color": "rgba(255, 0, 0, 0.5)"}, + )), + ("assert-css", ( + ".docblock .example-wrap.compile_fail", + {"border-left": "2px solid rgba(255, 0, 0, 0.5)"}, + )), + + ("move-cursor-to", ".docblock .example-wrap.compile_fail"), + + ("assert-css", ( + ".docblock .example-wrap.compile_fail .tooltip", + {"color": "rgb(255, 0, 0)"}, + )), + ("assert-css", ( + ".docblock .example-wrap.compile_fail", + {"border-left": "2px solid rgb(255, 0, 0)"}, + )), + + // should_panic block + ("assert-css", ( + ".docblock .example-wrap.should_panic .tooltip", + {"color": "rgba(255, 0, 0, 0.5)"}, + )), + ("assert-css", ( + ".docblock .example-wrap.should_panic", + {"border-left": "2px solid rgba(255, 0, 0, 0.5)"}, + )), + + ("move-cursor-to", ".docblock .example-wrap.should_panic"), + + ("assert-css", ( + ".docblock .example-wrap.should_panic .tooltip", + {"color": "rgb(255, 0, 0)"}, + )), + ("assert-css", ( + ".docblock .example-wrap.should_panic", + {"border-left": "2px solid rgb(255, 0, 0)"}, + )), + + // ignore block + ("assert-css", ( + ".docblock .example-wrap.ignore .tooltip", + {"color": "rgba(255, 142, 0, 0.6)"}, + )), + ("assert-css", ( + ".docblock .example-wrap.ignore", + {"border-left": "2px solid rgba(255, 142, 0, 0.6)"}, + )), + + ("move-cursor-to", ".docblock .example-wrap.ignore"), + + ("assert-css", ( + ".docblock .example-wrap.ignore .tooltip", + {"color": "rgb(255, 142, 0)"}, + )), + ("assert-css", ( + ".docblock .example-wrap.ignore", + {"border-left": "2px solid rgb(255, 142, 0)"}, + )), + ], +) + +call-function: ("check-colors", ("ayu")) +call-function: ("check-colors", ("dark")) +call-function: ("check-colors", ("light")) From e60016eb55ec312dfac4b33cbb509435a05919ed Mon Sep 17 00:00:00 2001 From: Mara Bos <m-ou.se@m-ou.se> Date: Wed, 19 Oct 2022 12:41:11 +0200 Subject: [PATCH 07/11] Split is_stable from rustc_target::spec::abi::is_enabled. --- compiler/rustc_target/src/spec/abi.rs | 268 +++++++++++--------------- 1 file changed, 109 insertions(+), 159 deletions(-) diff --git a/compiler/rustc_target/src/spec/abi.rs b/compiler/rustc_target/src/spec/abi.rs index c915124434b2d..ce45fa13970b4 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -109,175 +109,125 @@ pub enum AbiDisabled { Unrecognized, } -fn gate_feature_post( +pub fn is_enabled( features: &rustc_feature::Features, - feature: Symbol, span: Span, - explain: &'static str, + name: &str, ) -> Result<(), AbiDisabled> { - if !features.enabled(feature) && !span.allows_unstable(feature) { - Err(AbiDisabled::Unstable { feature, explain }) - } else { - Ok(()) + let s = is_stable(name); + if let Err(AbiDisabled::Unstable { feature, .. }) = s { + if features.enabled(feature) || span.allows_unstable(feature) { + return Ok(()); + } } + s } -pub fn is_enabled( - features: &rustc_feature::Features, - span: Span, - name: &str, -) -> Result<(), AbiDisabled> { +pub fn is_stable(name: &str) -> Result<(), AbiDisabled> { match name { // Stable "Rust" | "C" | "cdecl" | "stdcall" | "fastcall" | "aapcs" | "win64" | "sysv64" | "system" => Ok(()), - "rust-intrinsic" => { - gate_feature_post(features, sym::intrinsics, span, "intrinsics are subject to change") - } - "platform-intrinsic" => gate_feature_post( - features, - sym::platform_intrinsics, - span, - "platform intrinsics are experimental and possibly buggy", - ), - "vectorcall" => gate_feature_post( - features, - sym::abi_vectorcall, - span, - "vectorcall is experimental and subject to change", - ), - "thiscall" => gate_feature_post( - features, - sym::abi_thiscall, - span, - "thiscall is experimental and subject to change", - ), - "rust-call" => gate_feature_post( - features, - sym::unboxed_closures, - span, - "rust-call ABI is subject to change", - ), - "rust-cold" => gate_feature_post( - features, - sym::rust_cold_cc, - span, - "rust-cold is experimental and subject to change", - ), - "ptx-kernel" => gate_feature_post( - features, - sym::abi_ptx, - span, - "PTX ABIs are experimental and subject to change", - ), - "unadjusted" => gate_feature_post( - features, - sym::abi_unadjusted, - span, - "unadjusted ABI is an implementation detail and perma-unstable", - ), - "msp430-interrupt" => gate_feature_post( - features, - sym::abi_msp430_interrupt, - span, - "msp430-interrupt ABI is experimental and subject to change", - ), - "x86-interrupt" => gate_feature_post( - features, - sym::abi_x86_interrupt, - span, - "x86-interrupt ABI is experimental and subject to change", - ), - "amdgpu-kernel" => gate_feature_post( - features, - sym::abi_amdgpu_kernel, - span, - "amdgpu-kernel ABI is experimental and subject to change", - ), - "avr-interrupt" | "avr-non-blocking-interrupt" => gate_feature_post( - features, - sym::abi_avr_interrupt, - span, - "avr-interrupt and avr-non-blocking-interrupt ABIs are experimental and subject to change", - ), - "efiapi" => gate_feature_post( - features, - sym::abi_efiapi, - span, - "efiapi ABI is experimental and subject to change", - ), - "C-cmse-nonsecure-call" => gate_feature_post( - features, - sym::abi_c_cmse_nonsecure_call, - span, - "C-cmse-nonsecure-call ABI is experimental and subject to change", - ), - "C-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "C-unwind ABI is experimental and subject to change", - ), - "stdcall-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "stdcall-unwind ABI is experimental and subject to change", - ), - "system-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "system-unwind ABI is experimental and subject to change", - ), - "thiscall-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "thiscall-unwind ABI is experimental and subject to change", - ), - "cdecl-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "cdecl-unwind ABI is experimental and subject to change", - ), - "fastcall-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "fastcall-unwind ABI is experimental and subject to change", - ), - "vectorcall-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "vectorcall-unwind ABI is experimental and subject to change", - ), - "aapcs-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "aapcs-unwind ABI is experimental and subject to change", - ), - "win64-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "win64-unwind ABI is experimental and subject to change", - ), - "sysv64-unwind" => gate_feature_post( - features, - sym::c_unwind, - span, - "sysv64-unwind ABI is experimental and subject to change", - ), - "wasm" => gate_feature_post( - features, - sym::wasm_abi, - span, - "wasm ABI is experimental and subject to change", - ), + "rust-intrinsic" => Err(AbiDisabled::Unstable { + feature: sym::intrinsics, + explain: "intrinsics are subject to change", + }), + "platform-intrinsic" => Err(AbiDisabled::Unstable { + feature: sym::platform_intrinsics, + explain: "platform intrinsics are experimental and possibly buggy", + }), + "vectorcall" => Err(AbiDisabled::Unstable { + feature: sym::abi_vectorcall, + explain: "vectorcall is experimental and subject to change", + }), + "thiscall" => Err(AbiDisabled::Unstable { + feature: sym::abi_thiscall, + explain: "thiscall is experimental and subject to change", + }), + "rust-call" => Err(AbiDisabled::Unstable { + feature: sym::unboxed_closures, + explain: "rust-call ABI is subject to change", + }), + "rust-cold" => Err(AbiDisabled::Unstable { + feature: sym::rust_cold_cc, + explain: "rust-cold is experimental and subject to change", + }), + "ptx-kernel" => Err(AbiDisabled::Unstable { + feature: sym::abi_ptx, + explain: "PTX ABIs are experimental and subject to change", + }), + "unadjusted" => Err(AbiDisabled::Unstable { + feature: sym::abi_unadjusted, + explain: "unadjusted ABI is an implementation detail and perma-unstable", + }), + "msp430-interrupt" => Err(AbiDisabled::Unstable { + feature: sym::abi_msp430_interrupt, + explain: "msp430-interrupt ABI is experimental and subject to change", + }), + "x86-interrupt" => Err(AbiDisabled::Unstable { + feature: sym::abi_x86_interrupt, + explain: "x86-interrupt ABI is experimental and subject to change", + }), + "amdgpu-kernel" => Err(AbiDisabled::Unstable { + feature: sym::abi_amdgpu_kernel, + explain: "amdgpu-kernel ABI is experimental and subject to change", + }), + "avr-interrupt" | "avr-non-blocking-interrupt" => Err(AbiDisabled::Unstable { + feature: sym::abi_avr_interrupt, + explain: "avr-interrupt and avr-non-blocking-interrupt ABIs are experimental and subject to change", + }), + "efiapi" => Err(AbiDisabled::Unstable { + feature: sym::abi_efiapi, + explain: "efiapi ABI is experimental and subject to change", + }), + "C-cmse-nonsecure-call" => Err(AbiDisabled::Unstable { + feature: sym::abi_c_cmse_nonsecure_call, + explain: "C-cmse-nonsecure-call ABI is experimental and subject to change", + }), + "C-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "C-unwind ABI is experimental and subject to change", + }), + "stdcall-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "stdcall-unwind ABI is experimental and subject to change", + }), + "system-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "system-unwind ABI is experimental and subject to change", + }), + "thiscall-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "thiscall-unwind ABI is experimental and subject to change", + }), + "cdecl-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "cdecl-unwind ABI is experimental and subject to change", + }), + "fastcall-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "fastcall-unwind ABI is experimental and subject to change", + }), + "vectorcall-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "vectorcall-unwind ABI is experimental and subject to change", + }), + "aapcs-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "aapcs-unwind ABI is experimental and subject to change", + }), + "win64-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "win64-unwind ABI is experimental and subject to change", + }), + "sysv64-unwind" => Err(AbiDisabled::Unstable { + feature: sym::c_unwind, + explain: "sysv64-unwind ABI is experimental and subject to change", + }), + "wasm" => Err(AbiDisabled::Unstable { + feature: sym::wasm_abi, + explain: "wasm ABI is experimental and subject to change", + }), _ => Err(AbiDisabled::Unrecognized), } } From ead96f7f749846a2310e7aaedef0ea38fc7c3a4a Mon Sep 17 00:00:00 2001 From: Mara Bos <m-ou.se@m-ou.se> Date: Wed, 19 Oct 2022 12:41:35 +0200 Subject: [PATCH 08/11] Allow #[unstable] impls for fn() with unstable abi. --- compiler/rustc_passes/src/stability.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index cfd6acd8d7cd0..78591e640e38f 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -888,10 +888,15 @@ impl<'tcx> Visitor<'tcx> for CheckTraitImplStable<'tcx> { } fn visit_ty(&mut self, t: &'tcx Ty<'tcx>) { - if let TyKind::Never = t.kind { - self.fully_stable = false; + match t.kind { + TyKind::Never => self.fully_stable = false, + TyKind::BareFn(f) => { + if rustc_target::spec::abi::is_stable(f.abi.name()).is_err() { + self.fully_stable = false; + } + } + _ => intravisit::walk_ty(self, t), } - intravisit::walk_ty(self, t) } } From a44a22d5f42d2e81a649d0a019c02f85a803b92f Mon Sep 17 00:00:00 2001 From: Mara Bos <m-ou.se@m-ou.se> Date: Wed, 19 Oct 2022 12:41:56 +0200 Subject: [PATCH 09/11] Add test. --- .../stability-attribute-trait-impl.rs | 9 ++++++++- .../stability-attribute-trait-impl.stderr | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs b/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs index ce2726ffde487..48fb23ead3b45 100644 --- a/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs +++ b/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs @@ -1,4 +1,4 @@ -#![feature(staged_api)] +#![feature(staged_api, never_type, c_unwind)] //~^ ERROR module has missing stability attribute #[stable(feature = "a", since = "1")] @@ -23,6 +23,13 @@ impl StableTrait for UnstableType {} impl UnstableTrait for StableType {} #[unstable(feature = "h", issue = "none")] +impl StableTrait for ! {} + +// Note: If C-unwind is stabilized, switch this to another (unstable) ABI. +#[unstable(feature = "i", issue = "none")] +impl StableTrait for extern "C-unwind" fn() {} + +#[unstable(feature = "j", issue = "none")] //~^ ERROR an `#[unstable]` annotation here has no effect [ineffective_unstable_trait_impl] impl StableTrait for StableType {} diff --git a/src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr b/src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr index 7645f3c7ef514..037a48f7649a4 100644 --- a/src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr +++ b/src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr @@ -1,7 +1,7 @@ error: an `#[unstable]` annotation here has no effect - --> $DIR/stability-attribute-trait-impl.rs:25:1 + --> $DIR/stability-attribute-trait-impl.rs:32:1 | -LL | #[unstable(feature = "h", issue = "none")] +LL | #[unstable(feature = "j", issue = "none")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: see issue #55436 <https://github.com/rust-lang/rust/issues/55436> for more information @@ -10,7 +10,7 @@ LL | #[unstable(feature = "h", issue = "none")] error: module has missing stability attribute --> $DIR/stability-attribute-trait-impl.rs:1:1 | -LL | / #![feature(staged_api)] +LL | / #![feature(staged_api, never_type, c_unwind)] LL | | LL | | LL | | #[stable(feature = "a", since = "1")] From 5420fa388197ee61fc799ea22ea9bb820306dbb9 Mon Sep 17 00:00:00 2001 From: Mara Bos <m-ou.se@m-ou.se> Date: Wed, 19 Oct 2022 13:25:37 +0200 Subject: [PATCH 10/11] Add test for #[unstable] impl for fn() -> !. --- .../stability-attribute-trait-impl.rs | 4 ++++ .../stability-attribute-trait-impl.stderr | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs b/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs index 48fb23ead3b45..cc5bc3b6d9128 100644 --- a/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs +++ b/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs @@ -33,4 +33,8 @@ impl StableTrait for extern "C-unwind" fn() {} //~^ ERROR an `#[unstable]` annotation here has no effect [ineffective_unstable_trait_impl] impl StableTrait for StableType {} +#[unstable(feature = "k", issue = "none")] +//~^ ERROR an `#[unstable]` annotation here has no effect [ineffective_unstable_trait_impl] +impl StableTrait for fn() -> ! {} + fn main() {} diff --git a/src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr b/src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr index 037a48f7649a4..b91a1d2e11a8a 100644 --- a/src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr +++ b/src/test/ui/stability-attribute/stability-attribute-trait-impl.stderr @@ -7,6 +7,14 @@ LL | #[unstable(feature = "j", issue = "none")] = note: see issue #55436 <https://github.com/rust-lang/rust/issues/55436> for more information = note: `#[deny(ineffective_unstable_trait_impl)]` on by default +error: an `#[unstable]` annotation here has no effect + --> $DIR/stability-attribute-trait-impl.rs:36:1 + | +LL | #[unstable(feature = "k", issue = "none")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #55436 <https://github.com/rust-lang/rust/issues/55436> for more information + error: module has missing stability attribute --> $DIR/stability-attribute-trait-impl.rs:1:1 | @@ -19,5 +27,5 @@ LL | | LL | | fn main() {} | |____________^ -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors From c4f829b2e5053cef753e010fa89bcc01b164a306 Mon Sep 17 00:00:00 2001 From: Mara Bos <m-ou.se@m-ou.se> Date: Wed, 19 Oct 2022 13:33:45 +0200 Subject: [PATCH 11/11] Allow #[unstable] impl for fn() -> UnstableType. (But not fn() -> !, which is stable.) --- compiler/rustc_passes/src/stability.rs | 26 ++++++++++++++----- .../stability-attribute-trait-impl.rs | 3 +++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index 78591e640e38f..9591aeb881f3d 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -888,14 +888,26 @@ impl<'tcx> Visitor<'tcx> for CheckTraitImplStable<'tcx> { } fn visit_ty(&mut self, t: &'tcx Ty<'tcx>) { - match t.kind { - TyKind::Never => self.fully_stable = false, - TyKind::BareFn(f) => { - if rustc_target::spec::abi::is_stable(f.abi.name()).is_err() { - self.fully_stable = false; - } + if let TyKind::Never = t.kind { + self.fully_stable = false; + } + if let TyKind::BareFn(f) = t.kind { + if rustc_target::spec::abi::is_stable(f.abi.name()).is_err() { + self.fully_stable = false; + } + } + intravisit::walk_ty(self, t) + } + + fn visit_fn_decl(&mut self, fd: &'tcx hir::FnDecl<'tcx>) { + for ty in fd.inputs { + self.visit_ty(ty) + } + if let hir::FnRetTy::Return(output_ty) = fd.output { + match output_ty.kind { + TyKind::Never => {} // `-> !` is stable + _ => self.visit_ty(output_ty), } - _ => intravisit::walk_ty(self, t), } } } diff --git a/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs b/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs index cc5bc3b6d9128..0c771ae87953c 100644 --- a/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs +++ b/src/test/ui/stability-attribute/stability-attribute-trait-impl.rs @@ -37,4 +37,7 @@ impl StableTrait for StableType {} //~^ ERROR an `#[unstable]` annotation here has no effect [ineffective_unstable_trait_impl] impl StableTrait for fn() -> ! {} +#[unstable(feature = "l", issue = "none")] +impl StableTrait for fn() -> UnstableType {} + fn main() {}