From 8dc68ecdfcc764c7c0dcf5fcedcb51b092d99620 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 14 Jan 2021 18:25:19 +0300 Subject: [PATCH] Introduce more appropriate assertion mechanism rust-analyzer is a long-running program, so we *should* handle assertion failures. See also https://www.sqlite.org/assert.html. --- .../src/completions/qualified_path.rs | 3 +- .../src/completions/unqualified_path.rs | 6 +-- crates/completion/src/item.rs | 4 ++ crates/rust-analyzer/src/bin/main.rs | 5 ++ crates/stdx/src/lib.rs | 2 + crates/stdx/src/macros.rs | 52 +++++++++++++++++++ crates/syntax/src/display.rs | 2 +- docs/dev/style.md | 5 ++ 8 files changed, 72 insertions(+), 7 deletions(-) diff --git a/crates/completion/src/completions/qualified_path.rs b/crates/completion/src/completions/qualified_path.rs index fa9e6e8100..33df267615 100644 --- a/crates/completion/src/completions/qualified_path.rs +++ b/crates/completion/src/completions/qualified_path.rs @@ -590,8 +590,7 @@ fn main() { let _ = crate::$0 } "#, expect![[r##" fn main() fn main() - ma foo!(…) #[macro_export] - macro_rules! foo + ma foo!(…) #[macro_export] macro_rules! foo "##]], ); } diff --git a/crates/completion/src/completions/unqualified_path.rs b/crates/completion/src/completions/unqualified_path.rs index 7ba99447d7..53e1391f34 100644 --- a/crates/completion/src/completions/unqualified_path.rs +++ b/crates/completion/src/completions/unqualified_path.rs @@ -540,8 +540,7 @@ mod macros { "#, expect![[r##" fn f() fn f() - ma concat!(…) #[macro_export] - macro_rules! concat + ma concat!(…) #[macro_export] macro_rules! concat md std "##]], ); @@ -597,8 +596,7 @@ fn main() { let v = $0 } "#, expect![[r##" md m1 - ma baz!(…) #[macro_export] - macro_rules! baz + ma baz!(…) #[macro_export] macro_rules! baz fn main() fn main() md m2 ma bar!(…) macro_rules! bar diff --git a/crates/completion/src/item.rs b/crates/completion/src/item.rs index 35af354b0c..0134ff219b 100644 --- a/crates/completion/src/item.rs +++ b/crates/completion/src/item.rs @@ -7,6 +7,7 @@ use ide_db::helpers::{ insert_use::{self, ImportScope, MergeBehavior}, mod_path_to_ast, SnippetCap, }; +use stdx::assert_never; use syntax::{algo, TextRange}; use text_edit::TextEdit; @@ -396,6 +397,9 @@ impl Builder { } pub(crate) fn set_detail(mut self, detail: Option>) -> Builder { self.detail = detail.map(Into::into); + if let Some(detail) = &self.detail { + assert_never!(detail.contains('\n'), "multiline detail: {}", detail); + } self } #[allow(unused)] diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index 3af3c59d88..bf42654a88 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -70,6 +70,11 @@ fn setup_logging(log_file: Option) -> Result<()> { tracing_setup::setup_tracing()?; profile::init(); + + if !cfg!(debug_assertions) { + stdx::set_assert_hook(|loc, args| log::error!("assertion failed at {}: {}", loc, args)); + } + Ok(()) } diff --git a/crates/stdx/src/lib.rs b/crates/stdx/src/lib.rs index d9a62e943b..1ff2559bbd 100644 --- a/crates/stdx/src/lib.rs +++ b/crates/stdx/src/lib.rs @@ -4,6 +4,8 @@ use std::{cmp::Ordering, ops, process, time::Instant}; mod macros; pub mod panic_context; +pub use crate::macros::{on_assert_failure, set_assert_hook}; + #[inline(always)] pub fn is_ci() -> bool { option_env!("CI").is_some() diff --git a/crates/stdx/src/macros.rs b/crates/stdx/src/macros.rs index f5ee3484b0..263b938e3f 100644 --- a/crates/stdx/src/macros.rs +++ b/crates/stdx/src/macros.rs @@ -1,4 +1,9 @@ //! Convenience macros. + +use std::{ + fmt, mem, panic, + sync::atomic::{AtomicUsize, Ordering::SeqCst}, +}; #[macro_export] macro_rules! eprintln { ($($tt:tt)*) => {{ @@ -44,3 +49,50 @@ macro_rules! impl_from { )* } } + +/// A version of `assert!` macro which allows to handle an assertion failure. +/// +/// In release mode, it returns the condition and logs an error. +/// +/// ``` +/// if assert_never!(impossible) { +/// // Heh, this shouldn't have happened, but lets try to soldier on... +/// return None; +/// } +/// ``` +/// +/// Rust analyzer is a long-running process, and crashing really isn't an option. +/// +/// Shamelessly stolen from: https://www.sqlite.org/assert.html +#[macro_export] +macro_rules! assert_never { + ($cond:expr) => { $crate::assert_always!($cond, "") }; + ($cond:expr, $($fmt:tt)*) => {{ + let value = $cond; + if value { + $crate::on_assert_failure( + format_args!($($fmt)*) + ); + } + value + }}; +} + +type AssertHook = fn(&panic::Location<'_>, fmt::Arguments<'_>); +static HOOK: AtomicUsize = AtomicUsize::new(0); + +pub fn set_assert_hook(hook: AssertHook) { + HOOK.store(hook as usize, SeqCst); +} + +#[cold] +#[track_caller] +pub fn on_assert_failure(args: fmt::Arguments) { + let hook: usize = HOOK.load(SeqCst); + if hook == 0 { + panic!("\n assertion failed: {}\n", args); + } + + let hook: AssertHook = unsafe { mem::transmute::(hook) }; + hook(panic::Location::caller(), args) +} diff --git a/crates/syntax/src/display.rs b/crates/syntax/src/display.rs index 391647fc67..cd956d950d 100644 --- a/crates/syntax/src/display.rs +++ b/crates/syntax/src/display.rs @@ -80,7 +80,7 @@ pub fn macro_label(node: &ast::Macro) -> String { let name = node.name().map(|name| name.syntax().text().to_string()).unwrap_or_default(); match node { ast::Macro::MacroRules(node) => { - let vis = if node.has_atom_attr("macro_export") { "#[macro_export]\n" } else { "" }; + let vis = if node.has_atom_attr("macro_export") { "#[macro_export] " } else { "" }; format!("{}macro_rules! {}", vis, name) } ast::Macro::MacroDef(node) => { diff --git a/docs/dev/style.md b/docs/dev/style.md index 9859f61482..21330948ba 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -215,6 +215,11 @@ if idx >= len { **Rationale:** its useful to see the invariant relied upon by the rest of the function clearly spelled out. +## Assertions + +Assert liberally. +Prefer `stdx::assert_never!` to standard `assert!`. + ## Getters & Setters If a field can have any value without breaking invariants, make the field public.