From 6630c146f213088dff0d4ebd8dad90591544c169 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 23 Sep 2022 17:06:28 -0700 Subject: [PATCH 01/38] Implement the `+whole-archive` modifier for `wasm-ld` This implements the `Linker::{link_whole_staticlib,link_whole_rlib}` methods for the `WasmLd` linker used on wasm targets. Previously these methods were noops since I think historically `wasm-ld` did not have support for `--whole-archive` but nowadays it does, so the flags are passed through. --- compiler/rustc_codegen_ssa/src/back/linker.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index e0bd7a33f7373..6e59a4f64db25 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -1265,11 +1265,11 @@ impl<'a> Linker for WasmLd<'a> { } fn link_whole_staticlib(&mut self, lib: &str, _verbatim: bool, _search_path: &[PathBuf]) { - self.cmd.arg("-l").arg(lib); + self.cmd.arg("--whole-archive").arg("-l").arg(lib).arg("--no-whole-archive"); } fn link_whole_rlib(&mut self, lib: &Path) { - self.cmd.arg(lib); + self.cmd.arg("--whole-archive").arg(lib).arg("--no-whole-archive"); } fn gc_sections(&mut self, _keep_metadata: bool) { From c0447b489ba890c740c1e5c41f3eacca148734a9 Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 24 Oct 2022 16:48:28 +0800 Subject: [PATCH 02/38] fix #103435, unused lint won't produce invalid code --- compiler/rustc_lint/src/unused.rs | 44 +++++++++++++------ .../lint/issue-103435-extra-parentheses.fixed | 16 +++++++ .../ui/lint/issue-103435-extra-parentheses.rs | 16 +++++++ .../issue-103435-extra-parentheses.stderr | 43 ++++++++++++++++++ 4 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/lint/issue-103435-extra-parentheses.fixed create mode 100644 src/test/ui/lint/issue-103435-extra-parentheses.rs create mode 100644 src/test/ui/lint/issue-103435-extra-parentheses.stderr diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 46706e4984451..1299444cd7738 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -565,10 +565,26 @@ trait UnusedDelimLint { lint.set_arg("delim", Self::DELIM_STR); lint.set_arg("item", msg); if let Some((lo, hi)) = spans { - let replacement = vec![ - (lo, if keep_space.0 { " ".into() } else { "".into() }), - (hi, if keep_space.1 { " ".into() } else { "".into() }), - ]; + let sm = cx.sess().source_map(); + let lo_replace = + if keep_space.0 && + let Ok(snip) = sm.span_to_snippet(lo.with_lo(lo.lo() - BytePos(1))) && + !snip.starts_with(" ") { + " ".to_string() + } else { + "".to_string() + }; + + let hi_replace = + if keep_space.1 && + let Ok(snip) = sm.span_to_snippet(sm.next_point(hi)) && + !snip.starts_with(" ") { + " ".to_string() + } else { + "".to_string() + }; + + let replacement = vec![(lo, lo_replace), (hi, hi_replace)]; lint.multipart_suggestion( fluent::suggestion, replacement, @@ -765,6 +781,7 @@ impl UnusedParens { value: &ast::Pat, avoid_or: bool, avoid_mut: bool, + keep_space: (bool, bool), ) { use ast::{BindingAnnotation, PatKind}; @@ -789,7 +806,7 @@ impl UnusedParens { } else { None }; - self.emit_unused_delims(cx, value.span, spans, "pattern", (false, false)); + self.emit_unused_delims(cx, value.span, spans, "pattern", keep_space); } } } @@ -798,7 +815,7 @@ impl EarlyLintPass for UnusedParens { fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { match e.kind { ExprKind::Let(ref pat, _, _) | ExprKind::ForLoop(ref pat, ..) => { - self.check_unused_parens_pat(cx, pat, false, false); + self.check_unused_parens_pat(cx, pat, false, false, (true, true)); } // We ignore parens in cases like `if (((let Some(0) = Some(1))))` because we already // handle a hard error for them during AST lowering in `lower_expr_mut`, but we still @@ -842,6 +859,7 @@ impl EarlyLintPass for UnusedParens { fn check_pat(&mut self, cx: &EarlyContext<'_>, p: &ast::Pat) { use ast::{Mutability, PatKind::*}; + let keep_space = (false, false); match &p.kind { // Do not lint on `(..)` as that will result in the other arms being useless. Paren(_) @@ -849,33 +867,33 @@ impl EarlyLintPass for UnusedParens { | Wild | Rest | Lit(..) | MacCall(..) | Range(..) | Ident(.., None) | Path(..) => {}, // These are list-like patterns; parens can always be removed. TupleStruct(_, _, ps) | Tuple(ps) | Slice(ps) | Or(ps) => for p in ps { - self.check_unused_parens_pat(cx, p, false, false); + self.check_unused_parens_pat(cx, p, false, false, keep_space); }, Struct(_, _, fps, _) => for f in fps { - self.check_unused_parens_pat(cx, &f.pat, false, false); + self.check_unused_parens_pat(cx, &f.pat, false, false, keep_space); }, // Avoid linting on `i @ (p0 | .. | pn)` and `box (p0 | .. | pn)`, #64106. - Ident(.., Some(p)) | Box(p) => self.check_unused_parens_pat(cx, p, true, false), + Ident(.., Some(p)) | Box(p) => self.check_unused_parens_pat(cx, p, true, false, keep_space), // Avoid linting on `&(mut x)` as `&mut x` has a different meaning, #55342. // Also avoid linting on `& mut? (p0 | .. | pn)`, #64106. - Ref(p, m) => self.check_unused_parens_pat(cx, p, true, *m == Mutability::Not), + Ref(p, m) => self.check_unused_parens_pat(cx, p, true, *m == Mutability::Not, keep_space), } } fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { if let StmtKind::Local(ref local) = s.kind { - self.check_unused_parens_pat(cx, &local.pat, true, false); + self.check_unused_parens_pat(cx, &local.pat, true, false, (false, false)); } ::check_stmt(self, cx, s) } fn check_param(&mut self, cx: &EarlyContext<'_>, param: &ast::Param) { - self.check_unused_parens_pat(cx, ¶m.pat, true, false); + self.check_unused_parens_pat(cx, ¶m.pat, true, false, (false, false)); } fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) { - self.check_unused_parens_pat(cx, &arm.pat, false, false); + self.check_unused_parens_pat(cx, &arm.pat, false, false, (false, false)); } fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) { diff --git a/src/test/ui/lint/issue-103435-extra-parentheses.fixed b/src/test/ui/lint/issue-103435-extra-parentheses.fixed new file mode 100644 index 0000000000000..dbbcaa441ddd6 --- /dev/null +++ b/src/test/ui/lint/issue-103435-extra-parentheses.fixed @@ -0,0 +1,16 @@ +// run-rustfix +#![deny(unused_parens)] + +fn main() { + if let Some(_) = Some(1) {} + //~^ ERROR unnecessary parentheses around pattern + + for _x in 1..10 {} + //~^ ERROR unnecessary parentheses around pattern + + if 2 == 1 {} + //~^ ERROR unnecessary parentheses around `if` condition + + // FIXME, auto recover from this one? + // for(_x in 1..10) {} +} diff --git a/src/test/ui/lint/issue-103435-extra-parentheses.rs b/src/test/ui/lint/issue-103435-extra-parentheses.rs new file mode 100644 index 0000000000000..f5c2a6664ede4 --- /dev/null +++ b/src/test/ui/lint/issue-103435-extra-parentheses.rs @@ -0,0 +1,16 @@ +// run-rustfix +#![deny(unused_parens)] + +fn main() { + if let(Some(_))= Some(1) {} + //~^ ERROR unnecessary parentheses around pattern + + for(_x)in 1..10 {} + //~^ ERROR unnecessary parentheses around pattern + + if(2 == 1){} + //~^ ERROR unnecessary parentheses around `if` condition + + // FIXME, auto recover from this one? + // for(_x in 1..10) {} +} diff --git a/src/test/ui/lint/issue-103435-extra-parentheses.stderr b/src/test/ui/lint/issue-103435-extra-parentheses.stderr new file mode 100644 index 0000000000000..a3f2fbc51ab29 --- /dev/null +++ b/src/test/ui/lint/issue-103435-extra-parentheses.stderr @@ -0,0 +1,43 @@ +error: unnecessary parentheses around pattern + --> $DIR/issue-103435-extra-parentheses.rs:5:11 + | +LL | if let(Some(_))= Some(1) {} + | ^ ^ + | +note: the lint level is defined here + --> $DIR/issue-103435-extra-parentheses.rs:2:9 + | +LL | #![deny(unused_parens)] + | ^^^^^^^^^^^^^ +help: remove these parentheses + | +LL - if let(Some(_))= Some(1) {} +LL + if let Some(_) = Some(1) {} + | + +error: unnecessary parentheses around pattern + --> $DIR/issue-103435-extra-parentheses.rs:8:8 + | +LL | for(_x)in 1..10 {} + | ^ ^ + | +help: remove these parentheses + | +LL - for(_x)in 1..10 {} +LL + for _x in 1..10 {} + | + +error: unnecessary parentheses around `if` condition + --> $DIR/issue-103435-extra-parentheses.rs:11:7 + | +LL | if(2 == 1){} + | ^ ^ + | +help: remove these parentheses + | +LL - if(2 == 1){} +LL + if 2 == 1 {} + | + +error: aborting due to 3 previous errors + From a46af18cb1aedf7ccc5fbebd6f126c71da6cd4ee Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 24 Oct 2022 18:24:10 +0800 Subject: [PATCH 03/38] fix parentheses surrounding spacing issue in parser --- compiler/rustc_lint/src/unused.rs | 6 ++--- compiler/rustc_parse/src/errors.rs | 6 +++-- .../rustc_parse/src/parser/diagnostics.rs | 24 +++++++++++++++---- .../lint/issue-103435-extra-parentheses.fixed | 6 +++-- .../ui/lint/issue-103435-extra-parentheses.rs | 6 +++-- .../issue-103435-extra-parentheses.stderr | 20 +++++++++++++++- 6 files changed, 52 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 1299444cd7738..d4b083de27624 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -568,8 +568,7 @@ trait UnusedDelimLint { let sm = cx.sess().source_map(); let lo_replace = if keep_space.0 && - let Ok(snip) = sm.span_to_snippet(lo.with_lo(lo.lo() - BytePos(1))) && - !snip.starts_with(" ") { + let Ok(snip) = sm.span_to_prev_source(lo) && !snip.ends_with(" ") { " ".to_string() } else { "".to_string() @@ -577,8 +576,7 @@ trait UnusedDelimLint { let hi_replace = if keep_space.1 && - let Ok(snip) = sm.span_to_snippet(sm.next_point(hi)) && - !snip.starts_with(" ") { + let Ok(snip) = sm.span_to_next_source(hi) && !snip.starts_with(" ") { " ".to_string() } else { "".to_string() diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 9b177c5189bfb..a3fbc06293f70 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -1122,10 +1122,12 @@ pub(crate) struct ParenthesesInForHead { #[derive(Subdiagnostic)] #[multipart_suggestion(suggestion, applicability = "machine-applicable")] pub(crate) struct ParenthesesInForHeadSugg { - #[suggestion_part(code = "")] + #[suggestion_part(code = "{left_snippet}")] pub left: Span, - #[suggestion_part(code = "")] + pub left_snippet: String, + #[suggestion_part(code = "{right_snippet}")] pub right: Span, + pub right_snippet: String, } #[derive(Diagnostic)] diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 887a4a6de33b1..e9549e8999857 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1641,15 +1641,29 @@ impl<'a> Parser<'a> { (token::CloseDelim(Delimiter::Parenthesis), Some(begin_par_sp)) => { self.bump(); + let sm = self.sess.source_map(); + let left = begin_par_sp; + let right = self.prev_token.span; + let left_snippet = if let Ok(snip) = sm.span_to_prev_source(left) && + !snip.ends_with(" ") { + " ".to_string() + } else { + "".to_string() + }; + + let right_snippet = if let Ok(snip) = sm.span_to_next_source(right) && + !snip.starts_with(" ") { + " ".to_string() + } else { + "".to_string() + }; + self.sess.emit_err(ParenthesesInForHead { - span: vec![begin_par_sp, self.prev_token.span], + span: vec![left, right], // With e.g. `for (x) in y)` this would replace `(x) in y)` // with `x) in y)` which is syntactically invalid. // However, this is prevented before we get here. - sugg: ParenthesesInForHeadSugg { - left: begin_par_sp, - right: self.prev_token.span, - }, + sugg: ParenthesesInForHeadSugg { left, right, left_snippet, right_snippet }, }); // Unwrap `(pat)` into `pat` to avoid the `unused_parens` lint. diff --git a/src/test/ui/lint/issue-103435-extra-parentheses.fixed b/src/test/ui/lint/issue-103435-extra-parentheses.fixed index dbbcaa441ddd6..2b01b414baa6e 100644 --- a/src/test/ui/lint/issue-103435-extra-parentheses.fixed +++ b/src/test/ui/lint/issue-103435-extra-parentheses.fixed @@ -11,6 +11,8 @@ fn main() { if 2 == 1 {} //~^ ERROR unnecessary parentheses around `if` condition - // FIXME, auto recover from this one? - // for(_x in 1..10) {} + // reported by parser + for _x in 1..10 {} + //~^ ERROR expected one of + //~| ERROR unexpected parentheses surrounding } diff --git a/src/test/ui/lint/issue-103435-extra-parentheses.rs b/src/test/ui/lint/issue-103435-extra-parentheses.rs index f5c2a6664ede4..8261610cf5646 100644 --- a/src/test/ui/lint/issue-103435-extra-parentheses.rs +++ b/src/test/ui/lint/issue-103435-extra-parentheses.rs @@ -11,6 +11,8 @@ fn main() { if(2 == 1){} //~^ ERROR unnecessary parentheses around `if` condition - // FIXME, auto recover from this one? - // for(_x in 1..10) {} + // reported by parser + for(_x in 1..10){} + //~^ ERROR expected one of + //~| ERROR unexpected parentheses surrounding } diff --git a/src/test/ui/lint/issue-103435-extra-parentheses.stderr b/src/test/ui/lint/issue-103435-extra-parentheses.stderr index a3f2fbc51ab29..29c41c91050b9 100644 --- a/src/test/ui/lint/issue-103435-extra-parentheses.stderr +++ b/src/test/ui/lint/issue-103435-extra-parentheses.stderr @@ -1,3 +1,21 @@ +error: expected one of `)`, `,`, `@`, or `|`, found keyword `in` + --> $DIR/issue-103435-extra-parentheses.rs:15:12 + | +LL | for(_x in 1..10){} + | ^^ expected one of `)`, `,`, `@`, or `|` + +error: unexpected parentheses surrounding `for` loop head + --> $DIR/issue-103435-extra-parentheses.rs:15:8 + | +LL | for(_x in 1..10){} + | ^ ^ + | +help: remove parentheses in `for` loop + | +LL - for(_x in 1..10){} +LL + for _x in 1..10 {} + | + error: unnecessary parentheses around pattern --> $DIR/issue-103435-extra-parentheses.rs:5:11 | @@ -39,5 +57,5 @@ LL - if(2 == 1){} LL + if 2 == 1 {} | -error: aborting due to 3 previous errors +error: aborting due to 5 previous errors From 32a2f0dddb7aae3bef2939af5079eb2bcbfdf6c5 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 26 Oct 2022 00:15:47 +0800 Subject: [PATCH 04/38] suggest calling the method of the same name when method not found --- .../rustc_resolve/src/late/diagnostics.rs | 24 +++++++++---------- src/test/ui/resolve/issue-103474.rs | 16 +++++++++++++ src/test/ui/resolve/issue-103474.stderr | 20 ++++++++++++++++ src/test/ui/resolve/issue-2356.stderr | 4 ++-- src/test/ui/self/class-missing-self.stderr | 4 ++++ .../suggestions/assoc_fn_without_self.stderr | 9 +++++-- 6 files changed, 61 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/resolve/issue-103474.rs create mode 100644 src/test/ui/resolve/issue-103474.stderr diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 23c5fac8b71c1..23b9284db335f 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -213,26 +213,26 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { let (mod_prefix, mod_str, suggestion) = if path.len() == 1 { debug!(?self.diagnostic_metadata.current_impl_items); debug!(?self.diagnostic_metadata.current_function); - let suggestion = if let Some(items) = self.diagnostic_metadata.current_impl_items + let suggestion = if self.current_trait_ref.is_none() && let Some((fn_kind, _)) = self.diagnostic_metadata.current_function - && self.current_trait_ref.is_none() && let Some(FnCtxt::Assoc(_)) = fn_kind.ctxt() + && let Some(items) = self.diagnostic_metadata.current_impl_items && let Some(item) = items.iter().find(|i| { - if let AssocItemKind::Fn(fn_) = &i.kind - && !fn_.sig.decl.has_self() - && i.ident.name == item_str.name + if let AssocItemKind::Fn(_) = &i.kind && i.ident.name == item_str.name { debug!(?item_str.name); - debug!(?fn_.sig.decl.inputs); return true } false }) + && let AssocItemKind::Fn(fn_) = &item.kind { + debug!(?fn_); + let self_sugg = if fn_.sig.decl.has_self() { "self." } else { "Self::" }; Some(( - item_span, + item_span.shrink_to_lo(), "consider using the associated function", - format!("Self::{}", item.ident) + self_sugg.to_string() )) } else { None @@ -381,11 +381,13 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { } fn suggest_self_or_self_ref(&mut self, err: &mut Diagnostic, path: &[Segment], span: Span) { - let is_assoc_fn = self.self_type_is_available(); + if !self.self_type_is_available() { + return; + } let Some(path_last_segment) = path.last() else { return }; let item_str = path_last_segment.ident; // Emit help message for fake-self from other languages (e.g., `this` in Javascript). - if ["this", "my"].contains(&item_str.as_str()) && is_assoc_fn { + if ["this", "my"].contains(&item_str.as_str()) { err.span_suggestion_short( span, "you might have meant to use `self` here instead", @@ -436,7 +438,6 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { let is_enum_variant = &|res| matches!(res, Res::Def(DefKind::Variant, _)); let path_str = Segment::names_to_string(path); let ident_span = path.last().map_or(span, |ident| ident.ident.span); - let mut candidates = self .r .lookup_import_candidates(ident, ns, &self.parent_scope, is_expected) @@ -1512,7 +1513,6 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { _ => None, } } - // Fields are generally expected in the same contexts as locals. if filter_fn(Res::Local(ast::DUMMY_NODE_ID)) { if let Some(node_id) = diff --git a/src/test/ui/resolve/issue-103474.rs b/src/test/ui/resolve/issue-103474.rs new file mode 100644 index 0000000000000..408139ca0111e --- /dev/null +++ b/src/test/ui/resolve/issue-103474.rs @@ -0,0 +1,16 @@ +struct S {} +impl S { + fn first(&self) {} + + fn second(&self) { + first() + //~^ ERROR cannot find function `first` in this scope + } + + fn third(&self) { + no_method_err() + //~^ ERROR cannot find function `no_method_err` in this scope + } +} + +fn main() {} diff --git a/src/test/ui/resolve/issue-103474.stderr b/src/test/ui/resolve/issue-103474.stderr new file mode 100644 index 0000000000000..78fa13fbd2e24 --- /dev/null +++ b/src/test/ui/resolve/issue-103474.stderr @@ -0,0 +1,20 @@ +error[E0425]: cannot find function `first` in this scope + --> $DIR/issue-103474.rs:6:9 + | +LL | first() + | ^^^^^ not found in this scope + | +help: consider using the associated function + | +LL | self.first() + | +++++ + +error[E0425]: cannot find function `no_method_err` in this scope + --> $DIR/issue-103474.rs:11:9 + | +LL | no_method_err() + | ^^^^^^^^^^^^^ not found in this scope + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0425`. diff --git a/src/test/ui/resolve/issue-2356.stderr b/src/test/ui/resolve/issue-2356.stderr index e7c53ff44e6fe..36f3da7c95537 100644 --- a/src/test/ui/resolve/issue-2356.stderr +++ b/src/test/ui/resolve/issue-2356.stderr @@ -85,7 +85,7 @@ LL | static_method(); help: consider using the associated function | LL | Self::static_method(); - | ~~~~~~~~~~~~~~~~~~~ + | ++++++ error[E0425]: cannot find function `purr` in this scope --> $DIR/issue-2356.rs:54:9 @@ -114,7 +114,7 @@ LL | grow_older(); help: consider using the associated function | LL | Self::grow_older(); - | ~~~~~~~~~~~~~~~~ + | ++++++ error[E0425]: cannot find function `shave` in this scope --> $DIR/issue-2356.rs:74:5 diff --git a/src/test/ui/self/class-missing-self.stderr b/src/test/ui/self/class-missing-self.stderr index d501200d73cec..063c3f013c539 100644 --- a/src/test/ui/self/class-missing-self.stderr +++ b/src/test/ui/self/class-missing-self.stderr @@ -10,6 +10,10 @@ error[E0425]: cannot find function `sleep` in this scope LL | sleep(); | ^^^^^ not found in this scope | +help: consider using the associated function + | +LL | self.sleep(); + | +++++ help: consider importing this function | LL | use std::thread::sleep; diff --git a/src/test/ui/suggestions/assoc_fn_without_self.stderr b/src/test/ui/suggestions/assoc_fn_without_self.stderr index 88920b852905b..febdd67338c99 100644 --- a/src/test/ui/suggestions/assoc_fn_without_self.stderr +++ b/src/test/ui/suggestions/assoc_fn_without_self.stderr @@ -7,13 +7,18 @@ LL | foo(); help: consider using the associated function | LL | Self::foo(); - | ~~~~~~~~~ + | ++++++ error[E0425]: cannot find function `bar` in this scope --> $DIR/assoc_fn_without_self.rs:17:9 | LL | bar(); | ^^^ not found in this scope + | +help: consider using the associated function + | +LL | self.bar(); + | +++++ error[E0425]: cannot find function `baz` in this scope --> $DIR/assoc_fn_without_self.rs:18:9 @@ -24,7 +29,7 @@ LL | baz(2, 3); help: consider using the associated function | LL | Self::baz(2, 3); - | ~~~~~~~~~ + | ++++++ error[E0425]: cannot find function `foo` in this scope --> $DIR/assoc_fn_without_self.rs:14:13 From 27164495881d2d3d1bb1ef79850b00f1e9989ba7 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 26 Oct 2022 05:32:19 +0800 Subject: [PATCH 05/38] add testcase for suggest self --- src/test/ui/resolve/issue-103474.rs | 12 ++++++++++++ src/test/ui/resolve/issue-103474.stderr | 17 ++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/test/ui/resolve/issue-103474.rs b/src/test/ui/resolve/issue-103474.rs index 408139ca0111e..14f2259e1d4e8 100644 --- a/src/test/ui/resolve/issue-103474.rs +++ b/src/test/ui/resolve/issue-103474.rs @@ -13,4 +13,16 @@ impl S { } } +// https://github.com/rust-lang/rust/pull/103531#discussion_r1004728080 +struct Foo { + i: i32, +} + +impl Foo { + fn needs_self() { + this.i + //~^ ERROR cannot find value `this` in this scope + } +} + fn main() {} diff --git a/src/test/ui/resolve/issue-103474.stderr b/src/test/ui/resolve/issue-103474.stderr index 78fa13fbd2e24..415d231552a03 100644 --- a/src/test/ui/resolve/issue-103474.stderr +++ b/src/test/ui/resolve/issue-103474.stderr @@ -1,3 +1,18 @@ +error[E0425]: cannot find value `this` in this scope + --> $DIR/issue-103474.rs:23:9 + | +LL | this.i + | ^^^^ not found in this scope + | +help: you might have meant to use `self` here instead + | +LL | self.i + | ~~~~ +help: if you meant to use `self`, you are also missing a `self` receiver argument + | +LL | fn needs_self(&self) { + | +++++ + error[E0425]: cannot find function `first` in this scope --> $DIR/issue-103474.rs:6:9 | @@ -15,6 +30,6 @@ error[E0425]: cannot find function `no_method_err` in this scope LL | no_method_err() | ^^^^^^^^^^^^^ not found in this scope -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0425`. From 4f3a9881da89361358247ba58574b9eee37df4aa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Oct 2022 11:30:07 +0200 Subject: [PATCH 06/38] teach ./miri how to do Josh syncs --- src/tools/miri/CONTRIBUTING.md | 23 +++---- src/tools/miri/miri | 110 +++++++++++++++++++++++---------- 2 files changed, 89 insertions(+), 44 deletions(-) diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index b1e6b9c69d390..b18083849cf62 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -290,14 +290,12 @@ cargo run --release -p josh-proxy -- --local=$(pwd)/local --remote=https://githu ### Importing changes from the rustc repo +Josh needs to be running, as described above. We assume we start on an up-to-date master branch in the Miri repo. ```sh -# Fetch rustc side of the history. Takes ca 5 min the first time. -# Do NOT change that commit ID, it needs to be exactly this! -git fetch http://localhost:8000/rust-lang/rust.git:at_commit=75dd959a3a40eb5b4574f8d2e23aa6efbeb33573[:prefix=src/tools/miri]:/src/tools/miri.git master -# Include that history into ours. -git merge FETCH_HEAD -m "merge rustc history" +# Fetch and merge rustc side of the history. Takes ca 5 min the first time. +./miri rustc-pull # Update toolchain reference and apply formatting. ./rustup-toolchain HEAD && ./miri fmt git commit -am "rustup" @@ -310,16 +308,15 @@ needed. ### Exporting changes to the rustc repo -We will use the josh proxy to push to your fork of rustc. You need to make sure -that the master branch of your fork is up-to-date. Also make sure that there -exists no branch called `miri` in your fork. Then run the following in the Miri -repo, assuming we are on an up-to-date master branch: +Josh needs to be running, as described above. We will use the josh proxy to push +to your fork of rustc. Run the following in the Miri repo, assuming we are on an +up-to-date master branch: ```sh # Push the Miri changes to your rustc fork (substitute your github handle for YOUR_NAME). -# Do NOT change that commit ID, it needs to be exactly this! -git push http://localhost:8000/YOUR_NAME/rust.git:at_commit=75dd959a3a40eb5b4574f8d2e23aa6efbeb33573[:prefix=src/tools/miri]:/src/tools/miri.git -o base=master HEAD:miri +./miri rustc-push YOUR_NAME miri ``` -This will create a new branch in your fork, and the output should include a link -to create a rustc PR that will integrate those changes into the main repository. +This will create a new branch called 'miri' in your fork, and the output should +include a link to create a rustc PR that will integrate those changes into the +main repository. diff --git a/src/tools/miri/miri b/src/tools/miri/miri index e492308a62eb5..662f2b8e63172 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -42,6 +42,15 @@ many different seeds. Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. can explicitly list the benchmarks to run; by default, all of them are run. +./miri rustc-pull: +Pull and merge Miri changes from the rustc repo. + +./miri rustc-push : +Push Miri changes back to the rustc repo. This will update the 'master' branch +in the Rust fork of the given user to upstream. It will also pull a copy of the +rustc history into the Miri repo, unless you set the RUSTC_GIT env var to an +existing clone of the rustc repo. + ENVIRONMENT VARIABLES MIRI_SYSROOT: @@ -52,37 +61,60 @@ Pass extra flags to all cargo invocations. (Ignored by `./miri cargo`.) EOF ) -## We need to know where we are. +## We need to know which command to run and some global constants. +COMMAND="$1" +if [ -z "$COMMAND" ]; then + echo "$USAGE" + exit 1 +fi +shift # macOS does not have a useful readlink/realpath so we have to use Python instead... MIRIDIR=$(python3 -c 'import os, sys; print(os.path.dirname(os.path.realpath(sys.argv[1])))' "$0") +# Used for rustc syncs. +JOSH_FILTER=":at_commit=75dd959a3a40eb5b4574f8d2e23aa6efbeb33573[:prefix=src/tools/miri]:/src/tools/miri" -## Run the auto-things. -if [ -z "$MIRI_AUTO_OPS" ]; then - export MIRI_AUTO_OPS=42 - - # Run this first, so that the toolchain doesn't change after - # other code has run. - if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-toolchain" ] ; then - (cd "$MIRIDIR" && ./rustup-toolchain) +## Early commands, that don't do auto-things and don't want the environment-altering things happening below. +case "$COMMAND" in +rustc-pull) + cd "$MIRIDIR" + git fetch http://localhost:8000/rust-lang/rust.git$JOSH_FILTER.git master + git merge FETCH_HEAD + exit 0 + ;; +rustc-push) + USER="$1" + BRANCH="$2" + if [ -z "$USER" ] || [ -z "$BRANCH" ]; then + echo "Usage: $0 rustc-push " + exit 1 fi - - if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-fmt" ] ; then - $0 fmt + if [ -n "$RUSTC_GIT" ]; then + # Use an existing fork for the branch updates. + cd "$RUSTC_GIT" + else + # Do this in the local Miri repo. + echo "This will pull a copy of the rust-lang/rust history into this Miri checkout, growing it by about 1GB." + read -r -p "To avoid that, abort now and set the RUSTC_GIT environment variable to an existing rustc checkout. Proceed? [y/N] " + if [[ ! $REPLY =~ ^[Yy]$ ]]; then + exit 1 + fi + cd "$MIRIDIR" fi - - if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-clippy" ] ; then - $0 clippy -- -D warnings + # Prepare the branches. For reliable pushing we need to push to a non-existent branch + # and set `-o base` to a branch that holds current rustc master. + echo "Preparing $USER/rust..." + if git fetch https://github.com/$USER/rust $BRANCH &>/dev/null; then + echo "The '$BRANCH' seems to already exist in $USER/rust. Please delete it and try again." + exit 1 fi -fi - -## Determine command and toolchain. -COMMAND="$1" -[ $# -gt 0 ] && shift -# Doing this *after* auto-toolchain logic above, since that might change the toolchain. -TOOLCHAIN=$(cd "$MIRIDIR"; rustup show active-toolchain | head -n 1 | cut -d ' ' -f 1) - -## Handle some commands early, since they should *not* alter the environment. -case "$COMMAND" in + git fetch https://github.com/rust-lang/rust master + git push https://github.com/$USER/rust FETCH_HEAD:master + # Do the actual push. + cd "$MIRIDIR" + echo "Pushing Miri changes..." + git push http://localhost:8000/$USER/rust.git$JOSH_FILTER.git HEAD:$BRANCH -o base=master + exit 0 + ;; many-seeds) for SEED in $({ echo obase=16; seq 0 255; } | bc); do echo "Trying seed: $SEED" @@ -106,9 +138,28 @@ bench) ;; esac +## Run the auto-things. +if [ -z "$MIRI_AUTO_OPS" ]; then + export MIRI_AUTO_OPS=42 + + # Run this first, so that the toolchain doesn't change after + # other code has run. + if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-toolchain" ] ; then + (cd "$MIRIDIR" && ./rustup-toolchain) + fi + + if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-fmt" ] ; then + $0 fmt + fi + + if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-clippy" ] ; then + $0 clippy -- -D warnings + fi +fi + ## Prepare the environment # Determine some toolchain properties -# export the target so its available in miri +TOOLCHAIN=$(cd "$MIRIDIR"; rustup show active-toolchain | head -n 1 | cut -d ' ' -f 1) TARGET=$(rustc +$TOOLCHAIN --version --verbose | grep "^host:" | cut -d ' ' -f 2) SYSROOT=$(rustc +$TOOLCHAIN --print sysroot) LIBDIR=$SYSROOT/lib/rustlib/$TARGET/lib @@ -227,10 +278,7 @@ cargo) $CARGO "$@" ;; *) - if [ -n "$COMMAND" ]; then - echo "Unknown command: $COMMAND" - echo - fi - echo "$USAGE" + echo "Unknown command: $COMMAND" exit 1 + ;; esac From 39598e46f6777e0f5eee8177c347fec431d2b591 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Oct 2022 12:17:04 +0200 Subject: [PATCH 07/38] merge rustup-toolchain into ./miri --- src/tools/miri/.github/workflows/ci.yml | 6 +-- src/tools/miri/.gitpod.yml | 4 +- src/tools/miri/CONTRIBUTING.md | 10 ++--- src/tools/miri/README.md | 6 +-- src/tools/miri/miri | 45 ++++++++++++++++++++- src/tools/miri/rust-version | 2 +- src/tools/miri/rustup-toolchain | 53 ------------------------- 7 files changed, 58 insertions(+), 68 deletions(-) delete mode 100755 src/tools/miri/rustup-toolchain diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 3efb2d733d426..607ffe0cc59fe 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -67,9 +67,9 @@ jobs: shell: bash run: | if [[ ${{ github.event_name }} == 'schedule' ]]; then - ./rustup-toolchain HEAD --host ${{ matrix.host_target }} + ./miri toolchain HEAD --host ${{ matrix.host_target }} else - ./rustup-toolchain "" --host ${{ matrix.host_target }} + ./miri toolchain "" --host ${{ matrix.host_target }} fi - name: Show Rust version @@ -118,7 +118,7 @@ jobs: - name: Install "master" toolchain shell: bash run: | - ./rustup-toolchain "" -c clippy + ./miri toolchain - name: Show Rust version run: | diff --git a/src/tools/miri/.gitpod.yml b/src/tools/miri/.gitpod.yml index 36bd991740a82..724cf26df2b9b 100644 --- a/src/tools/miri/.gitpod.yml +++ b/src/tools/miri/.gitpod.yml @@ -4,6 +4,6 @@ tasks: - before: echo "..." init: | cargo install rustup-toolchain-install-master - ./rustup-toolchain + ./miri toolchain ./miri build - command: echo "Run tests with ./miri test" \ No newline at end of file + command: echo "Run tests with ./miri test" diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index b18083849cf62..3e36c2ea5f97c 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -23,13 +23,13 @@ tested against. Other versions will likely not work. After installing [`rustup-toolchain-install-master`], you can run the following command to install that exact version of rustc as a toolchain: ``` -./rustup-toolchain +./miri toolchain ``` This will set up a rustup toolchain called `miri` and set it as an override for the current directory. You can also create a `.auto-everything` file (contents don't matter, can be empty), which -will cause any `./miri` command to automatically call `rustup-toolchain`, `clippy` and `rustfmt` +will cause any `./miri` command to automatically call `./miri toolchain`, `clippy` and `rustfmt` for you. If you don't want all of these to happen, you can add individual `.auto-toolchain`, `.auto-clippy` and `.auto-fmt` files respectively. @@ -132,7 +132,7 @@ development version of Miri using and then you can use it as if it was installed by `rustup`. Make sure you use the same toolchain when calling `cargo miri` that you used when installing Miri! Usually this means you have to write `cargo +miri miri ...` to select the `miri` -toolchain that was installed by `./rustup-toolchain`. +toolchain that was installed by `./miri toolchain`. There's a test for the cargo wrapper in the `test-cargo-miri` directory; run `./run-test.py` in there to execute it. Like `./miri test`, this respects the @@ -214,7 +214,7 @@ for changes in rustc. In both cases, `rustc-version` needs updating. To update the `rustc-version` file and install the latest rustc, you can run: ``` -./rustup-toolchain HEAD +./miri toolchain HEAD ``` Now edit Miri until `./miri test` passes, and submit a PR. Generally, it is @@ -297,7 +297,7 @@ We assume we start on an up-to-date master branch in the Miri repo. # Fetch and merge rustc side of the history. Takes ca 5 min the first time. ./miri rustc-pull # Update toolchain reference and apply formatting. -./rustup-toolchain HEAD && ./miri fmt +./miri toolchain HEAD && ./miri fmt git commit -am "rustup" ``` diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 5803a88c0e757..1b84d188c55db 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -419,9 +419,9 @@ Some native rustc `-Z` flags are also very relevant for Miri: Moreover, Miri recognizes some environment variables: -* `MIRI_AUTO_OPS` indicates whether the automatic execution of rustfmt, clippy and rustup-toolchain - should be skipped. If it is set to any value, they are skipped. This is used for avoiding - infinite recursion in `./miri` and to allow automated IDE actions to avoid the auto ops. +* `MIRI_AUTO_OPS` indicates whether the automatic execution of rustfmt, clippy and toolchain setup + should be skipped. If it is set to any value, they are skipped. This is used for avoiding infinite + recursion in `./miri` and to allow automated IDE actions to avoid the auto ops. * `MIRI_LOG`, `MIRI_BACKTRACE` control logging and backtrace printing during Miri executions, also [see "Testing the Miri driver" in `CONTRIBUTING.md`][testing-miri]. * `MIRIFLAGS` (recognized by `cargo miri` and the test suite) defines extra diff --git a/src/tools/miri/miri b/src/tools/miri/miri index 662f2b8e63172..52902410a6b2d 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -51,6 +51,13 @@ in the Rust fork of the given user to upstream. It will also pull a copy of the rustc history into the Miri repo, unless you set the RUSTC_GIT env var to an existing clone of the rustc repo. +./miri toolchain : +Update and activate the rustup toolchain 'miri'. If no commit is given, updates +to the commit given in the `rust-version` file. If the commit is `HEAD`, updates +to the latest upstream rustc commit. +`rustup-toolchain-install-master` must be installed for this to work. Any extra +flags are passed to `rustup-toolchain-install-master`. + ENVIRONMENT VARIABLES MIRI_SYSROOT: @@ -75,6 +82,42 @@ JOSH_FILTER=":at_commit=75dd959a3a40eb5b4574f8d2e23aa6efbeb33573[:prefix=src/too ## Early commands, that don't do auto-things and don't want the environment-altering things happening below. case "$COMMAND" in +toolchain) + cd "$MIRIDIR" + # Make sure rustup-toolchain-install-master is installed. + if ! which rustup-toolchain-install-master >/dev/null; then + echo "Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'" + exit 1 + fi + # Determine new commit. + if [[ "$1" == "" ]]; then + NEW_COMMIT=$(cat rust-version) + elif [[ "$1" == "HEAD" ]]; then + NEW_COMMIT=$(git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1) + else + NEW_COMMIT="$1" + fi + echo "$NEW_COMMIT" > rust-version + shift || true # don't fail if shifting fails because no commit was given + # Check if we already are at that commit. + CUR_COMMIT=$(rustc +miri --version -v 2>/dev/null | grep "^commit-hash: " | cut -d " " -f 2) + if [[ "$CUR_COMMIT" == "$NEW_COMMIT" ]]; then + echo "miri toolchain is already at commit $CUR_COMMIT." + rustup override set miri + exit 0 + fi + # Install and setup new toolchain. + rustup toolchain uninstall miri + rustup-toolchain-install-master -n miri -c cargo -c rust-src -c rustc-dev -c llvm-tools -c rustfmt -c clippy "$@" -- "$NEW_COMMIT" + rustup override set miri + # Cleanup. + cargo clean + # Call 'cargo metadata' on the sources in case that changes the lockfile + # (which fails under some setups when it is done from inside vscode). + cargo metadata --format-version 1 --manifest-path "$(rustc --print sysroot)/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml" >/dev/null + # Done! + exit 0 + ;; rustc-pull) cd "$MIRIDIR" git fetch http://localhost:8000/rust-lang/rust.git$JOSH_FILTER.git master @@ -145,7 +188,7 @@ if [ -z "$MIRI_AUTO_OPS" ]; then # Run this first, so that the toolchain doesn't change after # other code has run. if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-toolchain" ] ; then - (cd "$MIRIDIR" && ./rustup-toolchain) + $0 toolchain fi if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-fmt" ] ; then diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index d0e98a8b0dba9..e582bf35356d6 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -85d089b41e2a0c0f07ab34f6c5a7c451389f25e6 +607878d069267e1402ad792c9331b426e4c6d0f9 diff --git a/src/tools/miri/rustup-toolchain b/src/tools/miri/rustup-toolchain deleted file mode 100755 index d7730f2b06d36..0000000000000 --- a/src/tools/miri/rustup-toolchain +++ /dev/null @@ -1,53 +0,0 @@ -#!/bin/bash -set -e -# Manages a rustup toolchain called "miri". -# -# All commands set "miri" as the override toolchain for the current directory, -# and make the `rust-version` file match that toolchain. -# -# USAGE: -# -# ./rustup-toolchain: Update "miri" toolchain to match `rust-version` (the known-good version for this commit). -# -# ./rustup-toolchain HEAD: Update "miri" toolchain and `rust-version` file to latest rustc HEAD. -# -# ./rustup-toolchain $COMMIT: Update "miri" toolchain and `rust-version` file to match that commit. -# -# Any extra parameters are passed to `rustup-toolchain-install-master`. - -# Make sure rustup-toolchain-install-master is installed. -if ! which rustup-toolchain-install-master >/dev/null; then - echo "Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'" - exit 1 -fi - -# Determine new commit. -if [[ "$1" == "" ]]; then - NEW_COMMIT=$(cat rust-version) -elif [[ "$1" == "HEAD" ]]; then - NEW_COMMIT=$(git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1) -else - NEW_COMMIT="$1" -fi -echo "$NEW_COMMIT" > rust-version -shift || true # don't fail if shifting fails - -# Check if we already are at that commit. -CUR_COMMIT=$(rustc +miri --version -v 2>/dev/null | grep "^commit-hash: " | cut -d " " -f 2) -if [[ "$CUR_COMMIT" == "$NEW_COMMIT" ]]; then - echo "miri toolchain is already at commit $CUR_COMMIT." - rustup override set miri - exit 0 -fi - -# Install and setup new toolchain. -rustup toolchain uninstall miri -rustup-toolchain-install-master -n miri -c cargo -c rust-src -c rustc-dev -c llvm-tools -c rustfmt -c clippy "$@" -- "$NEW_COMMIT" -rustup override set miri - -# Cleanup. -cargo clean - -# Call 'cargo metadata' on the sources in case that changes the lockfile -# (which fails under some setups when it is done from inside vscode). -cargo metadata --format-version 1 --manifest-path "$(rustc --print sysroot)/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml" >/dev/null From 2a3a53b1514a5e87d4992175274d2599e63c0734 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Oct 2022 12:36:31 +0200 Subject: [PATCH 08/38] explain how to go back to rustup-managed Miri --- src/tools/miri/CONTRIBUTING.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index b1e6b9c69d390..a2c86c3a8b23e 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -138,6 +138,9 @@ There's a test for the cargo wrapper in the `test-cargo-miri` directory; run `./run-test.py` in there to execute it. Like `./miri test`, this respects the `MIRI_TEST_TARGET` environment variable to execute the test for another target. +Note that installing Miri like this will "take away" Miri management from `rustup`. +If you want to later go back to a rustup-installed Miri, run `rustup update`. + ### Using a modified standard library Miri re-builds the standard library into a custom sysroot, so it is fairly easy From 1470e9924460b299ce597ef9001e5e3f7549a8b8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Oct 2022 16:11:03 +0200 Subject: [PATCH 09/38] Stacked Borrows: make scalar field retagging the default --- src/tools/miri/README.md | 7 ++++--- src/tools/miri/src/eval.rs | 2 +- .../stacked_borrows/newtype_pair_retagging.rs | 1 - .../fail/stacked_borrows/newtype_retagging.rs | 1 - .../return_invalid_mut_option.rs | 5 ++--- .../return_invalid_mut_option.stderr | 19 ++++++++++++------- .../return_invalid_mut_tuple.rs | 5 ++--- .../return_invalid_mut_tuple.stderr | 13 +++++++++---- .../return_invalid_shr_option.rs | 5 ++--- .../return_invalid_shr_option.stderr | 19 ++++++++++++------- .../return_invalid_shr_tuple.rs | 5 ++--- .../return_invalid_shr_tuple.stderr | 13 +++++++++---- .../stacked-borrows/no_field_retagging.rs | 19 +++++++++++++++++++ .../stacked-borrows/stack-printing.stdout | 4 ++-- 14 files changed, 76 insertions(+), 42 deletions(-) create mode 100644 src/tools/miri/tests/pass/stacked-borrows/no_field_retagging.rs diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 1b84d188c55db..7b684d5df1370 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -374,14 +374,15 @@ to Miri failing to detect cases of undefined behavior in a program. application instead of raising an error within the context of Miri (and halting execution). Note that code might not expect these operations to ever panic, so this flag can lead to strange (mis)behavior. -* `-Zmiri-retag-fields` changes Stacked Borrows retagging to recurse into fields. +* `-Zmiri-retag-fields` changes Stacked Borrows retagging to recurse into *all* fields. This means that references in fields of structs/enums/tuples/arrays/... are retagged, and in particular, they are protected when passed as function arguments. + (The default is to recurse only in cases where rustc would actually emit a `noalias` attribute.) * `-Zmiri-retag-fields=` controls when Stacked Borrows retagging recurses into fields. `all` means it always recurses (like `-Zmiri-retag-fields`), `none` means it never - recurses (the default), `scalar` means it only recurses for types where we would also emit + recurses, `scalar` (the default) means it only recurses for types where we would also emit `noalias` annotations in the generated LLVM IR (types passed as indivudal scalars or pairs of - scalars). + scalars). Setting this to `none` is **unsound**. * `-Zmiri-tag-gc=` configures how often the pointer tag garbage collector runs. The default is to search for and remove unreachable tags once every `10000` basic blocks. Setting this to `0` disables the garbage collector, which causes some programs to have explosive memory usage diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index a3fc343f8b67c..81132db94cf18 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -163,7 +163,7 @@ impl Default for MiriConfig { mute_stdout_stderr: false, preemption_rate: 0.01, // 1% report_progress: None, - retag_fields: RetagFields::No, + retag_fields: RetagFields::OnlyScalar, external_so_file: None, gc_interval: 10_000, num_cpus: 1, diff --git a/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.rs b/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.rs index 5cefdb08e7879..cc774500a3c69 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.rs @@ -1,4 +1,3 @@ -//@compile-flags: -Zmiri-retag-fields=scalar //@error-pattern: which is protected struct Newtype<'a>(&'a mut i32, i32); diff --git a/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.rs b/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.rs index bc3883575c333..1aa6e240e30f1 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.rs @@ -1,4 +1,3 @@ -//@compile-flags: -Zmiri-retag-fields=scalar //@error-pattern: which is protected struct Newtype<'a>(&'a mut i32); diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.rs b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.rs index 7fa9cf77d44b2..5a9dc6afba8da 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.rs @@ -1,16 +1,15 @@ // Make sure that we cannot return a `&mut` that got already invalidated, not even in an `Option`. -// Due to shallow reborrowing, the error only surfaces when we look into the `Option`. fn foo(x: &mut (i32, i32)) -> Option<&mut i32> { let xraw = x as *mut (i32, i32); let ret = unsafe { &mut (*xraw).1 }; // let-bind to avoid 2phase let ret = Some(ret); let _val = unsafe { *xraw }; // invalidate xref - ret + ret //~ ERROR: /retag .* tag does not exist in the borrow stack/ } fn main() { match foo(&mut (1, 2)) { - Some(_x) => {} //~ ERROR: /retag .* tag does not exist in the borrow stack/ + Some(_x) => {} None => {} } } diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr index 1068c286c62fa..c0ff35ebcde30 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr @@ -1,11 +1,11 @@ error: Undefined Behavior: trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location --> $DIR/return_invalid_mut_option.rs:LL:CC | -LL | Some(_x) => {} - | ^^ - | | - | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] +LL | ret + | ^^^ + | | + | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location + | this error occurs as part of retag at ALLOC[0x4..0x8] | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information @@ -13,14 +13,19 @@ help: was created by a Unique retag at offsets [0x4..0x8] --> $DIR/return_invalid_mut_option.rs:LL:CC | LL | let ret = Some(ret); - | ^^^ + | ^^^^^^^^^ help: was later invalidated at offsets [0x0..0x8] by a read access --> $DIR/return_invalid_mut_option.rs:LL:CC | LL | let _val = unsafe { *xraw }; // invalidate xref | ^^^^^ = note: BACKTRACE: - = note: inside `main` at $DIR/return_invalid_mut_option.rs:LL:CC + = note: inside `foo` at $DIR/return_invalid_mut_option.rs:LL:CC +note: inside `main` at $DIR/return_invalid_mut_option.rs:LL:CC + --> $DIR/return_invalid_mut_option.rs:LL:CC + | +LL | match foo(&mut (1, 2)) { + | ^^^^^^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.rs b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.rs index c94fef90542fd..8fe7f15cab0c1 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.rs @@ -1,12 +1,11 @@ // Make sure that we cannot return a `&mut` that got already invalidated, not even in a tuple. -// Due to shallow reborrowing, the error only surfaces when we look into the tuple. fn foo(x: &mut (i32, i32)) -> (&mut i32,) { let xraw = x as *mut (i32, i32); let ret = (unsafe { &mut (*xraw).1 },); let _val = unsafe { *xraw }; // invalidate xref - ret + ret //~ ERROR: /retag .* tag does not exist in the borrow stack/ } fn main() { - foo(&mut (1, 2)).0; //~ ERROR: /retag .* tag does not exist in the borrow stack/ + foo(&mut (1, 2)).0; } diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr index 79de9b668cf2b..9abf43c29f08f 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr @@ -1,8 +1,8 @@ error: Undefined Behavior: trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location --> $DIR/return_invalid_mut_tuple.rs:LL:CC | -LL | foo(&mut (1, 2)).0; - | ^^^^^^^^^^^^^^^^^^ +LL | ret + | ^^^ | | | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag at ALLOC[0x4..0x8] @@ -13,14 +13,19 @@ help: was created by a Unique retag at offsets [0x4..0x8] --> $DIR/return_invalid_mut_tuple.rs:LL:CC | LL | let ret = (unsafe { &mut (*xraw).1 },); - | ^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: was later invalidated at offsets [0x0..0x8] by a read access --> $DIR/return_invalid_mut_tuple.rs:LL:CC | LL | let _val = unsafe { *xraw }; // invalidate xref | ^^^^^ = note: BACKTRACE: - = note: inside `main` at $DIR/return_invalid_mut_tuple.rs:LL:CC + = note: inside `foo` at $DIR/return_invalid_mut_tuple.rs:LL:CC +note: inside `main` at $DIR/return_invalid_mut_tuple.rs:LL:CC + --> $DIR/return_invalid_mut_tuple.rs:LL:CC + | +LL | foo(&mut (1, 2)).0; + | ^^^^^^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_option.rs b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_option.rs index 3a028ceed86ae..094ce33b9c1f7 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_option.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_option.rs @@ -1,15 +1,14 @@ // Make sure that we cannot return a `&` that got already invalidated, not even in an `Option`. -// Due to shallow reborrowing, the error only surfaces when we look into the `Option`. fn foo(x: &mut (i32, i32)) -> Option<&i32> { let xraw = x as *mut (i32, i32); let ret = Some(unsafe { &(*xraw).1 }); unsafe { *xraw = (42, 23) }; // unfreeze - ret + ret //~ ERROR: /retag .* tag does not exist in the borrow stack/ } fn main() { match foo(&mut (1, 2)) { - Some(_x) => {} //~ ERROR: /retag .* tag does not exist in the borrow stack/ + Some(_x) => {} None => {} } } diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_option.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_option.stderr index f45456305db29..6066bf89f5d09 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_option.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_option.stderr @@ -1,11 +1,11 @@ error: Undefined Behavior: trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location --> $DIR/return_invalid_shr_option.rs:LL:CC | -LL | Some(_x) => {} - | ^^ - | | - | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] +LL | ret + | ^^^ + | | + | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location + | this error occurs as part of retag at ALLOC[0x4..0x8] | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information @@ -13,14 +13,19 @@ help: was created by a SharedReadOnly retag at offsets [0x4..0x8] --> $DIR/return_invalid_shr_option.rs:LL:CC | LL | let ret = Some(unsafe { &(*xraw).1 }); - | ^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: was later invalidated at offsets [0x0..0x8] by a write access --> $DIR/return_invalid_shr_option.rs:LL:CC | LL | unsafe { *xraw = (42, 23) }; // unfreeze | ^^^^^^^^^^^^^^^^ = note: BACKTRACE: - = note: inside `main` at $DIR/return_invalid_shr_option.rs:LL:CC + = note: inside `foo` at $DIR/return_invalid_shr_option.rs:LL:CC +note: inside `main` at $DIR/return_invalid_shr_option.rs:LL:CC + --> $DIR/return_invalid_shr_option.rs:LL:CC + | +LL | match foo(&mut (1, 2)) { + | ^^^^^^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_tuple.rs b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_tuple.rs index e4536626dbf2c..d0fd53e06aa26 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_tuple.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_tuple.rs @@ -1,12 +1,11 @@ // Make sure that we cannot return a `&` that got already invalidated, not even in a tuple. -// Due to shallow reborrowing, the error only surfaces when we look into the tuple. fn foo(x: &mut (i32, i32)) -> (&i32,) { let xraw = x as *mut (i32, i32); let ret = (unsafe { &(*xraw).1 },); unsafe { *xraw = (42, 23) }; // unfreeze - ret + ret //~ ERROR: /retag .* tag does not exist in the borrow stack/ } fn main() { - foo(&mut (1, 2)).0; //~ ERROR: /retag .* tag does not exist in the borrow stack/ + foo(&mut (1, 2)).0; } diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_tuple.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_tuple.stderr index 2e41f505bb9d2..52d365246a744 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_tuple.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_shr_tuple.stderr @@ -1,8 +1,8 @@ error: Undefined Behavior: trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location --> $DIR/return_invalid_shr_tuple.rs:LL:CC | -LL | foo(&mut (1, 2)).0; - | ^^^^^^^^^^^^^^^^^^ +LL | ret + | ^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag at ALLOC[0x4..0x8] @@ -13,14 +13,19 @@ help: was created by a SharedReadOnly retag at offsets [0x4..0x8] --> $DIR/return_invalid_shr_tuple.rs:LL:CC | LL | let ret = (unsafe { &(*xraw).1 },); - | ^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: was later invalidated at offsets [0x0..0x8] by a write access --> $DIR/return_invalid_shr_tuple.rs:LL:CC | LL | unsafe { *xraw = (42, 23) }; // unfreeze | ^^^^^^^^^^^^^^^^ = note: BACKTRACE: - = note: inside `main` at $DIR/return_invalid_shr_tuple.rs:LL:CC + = note: inside `foo` at $DIR/return_invalid_shr_tuple.rs:LL:CC +note: inside `main` at $DIR/return_invalid_shr_tuple.rs:LL:CC + --> $DIR/return_invalid_shr_tuple.rs:LL:CC + | +LL | foo(&mut (1, 2)).0; + | ^^^^^^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/pass/stacked-borrows/no_field_retagging.rs b/src/tools/miri/tests/pass/stacked-borrows/no_field_retagging.rs new file mode 100644 index 0000000000000..48fc8e8668ce0 --- /dev/null +++ b/src/tools/miri/tests/pass/stacked-borrows/no_field_retagging.rs @@ -0,0 +1,19 @@ +//@compile-flags: -Zmiri-retag-fields=none + +struct Newtype<'a>(&'a mut i32); + +fn dealloc_while_running(_n: Newtype<'_>, dealloc: impl FnOnce()) { + dealloc(); +} + +// Make sure that we do *not* retag the fields of `Newtype`. +fn main() { + let ptr = Box::into_raw(Box::new(0i32)); + #[rustfmt::skip] // I like my newlines + unsafe { + dealloc_while_running( + Newtype(&mut *ptr), + || drop(Box::from_raw(ptr)), + ) + }; +} diff --git a/src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout b/src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout index 838733078209d..296339e738455 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout +++ b/src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout @@ -1,6 +1,6 @@ 0..1: [ SharedReadWrite ] 0..1: [ SharedReadWrite ] 0..1: [ SharedReadWrite ] -0..1: [ SharedReadWrite Unique Unique Unique Unique Unique ] -0..1: [ SharedReadWrite Disabled Disabled Disabled Disabled Disabled SharedReadOnly ] +0..1: [ SharedReadWrite Unique Unique Unique Unique Unique Unique Unique ] +0..1: [ SharedReadWrite Disabled Disabled Disabled Disabled Disabled Disabled Disabled SharedReadOnly ] 0..1: [ unknown-bottom(..) ] From 0b49a5d17354c3b0980c3148f8291e429fc9a194 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Oct 2022 09:10:45 +0100 Subject: [PATCH 10/38] rustup --- src/tools/miri/miri | 2 +- src/tools/miri/rust-version | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/miri b/src/tools/miri/miri index 52902410a6b2d..a0593deb08a37 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -121,7 +121,7 @@ toolchain) rustc-pull) cd "$MIRIDIR" git fetch http://localhost:8000/rust-lang/rust.git$JOSH_FILTER.git master - git merge FETCH_HEAD + git merge FETCH_HEAD --no-ff -m "Merge from rustc" exit 0 ;; rustc-push) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index e582bf35356d6..13492d183c999 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -607878d069267e1402ad792c9331b426e4c6d0f9 +b03502b35d111bef0399a66ab3cc765f0802e8ba From 41c368ba8b0ca08c05ad7bf4a2c3dbbbfe9168ff Mon Sep 17 00:00:00 2001 From: Rageking8 Date: Mon, 31 Oct 2022 12:41:26 +0800 Subject: [PATCH 11/38] fix dupe word typos --- src/tools/miri/cargo-miri/src/phases.rs | 2 +- src/tools/miri/src/stacked_borrows/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 22da80be90211..2c84165a0f0a1 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -528,7 +528,7 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner cmd.args(binary_args); // Make sure we use the build-time working directory for interpreting Miri/rustc arguments. - // But then we need to switch to the run-time one, which we instruct Miri do do by setting `MIRI_CWD`. + // But then we need to switch to the run-time one, which we instruct Miri do by setting `MIRI_CWD`. cmd.current_dir(info.current_dir); cmd.env("MIRI_CWD", env::current_dir().unwrap()); diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index 5ec787dd44113..7f18e5dbae052 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -252,7 +252,7 @@ pub fn err_sb_ub<'tcx>( /// We need to make at least the following things true: /// /// U1: After creating a `Uniq`, it is at the top. -/// U2: If the top is `Uniq`, accesses must be through that `Uniq` or remove it it. +/// U2: If the top is `Uniq`, accesses must be through that `Uniq` or remove it. /// U3: If an access happens with a `Uniq`, it requires the `Uniq` to be in the stack. /// /// F1: After creating a `&`, the parts outside `UnsafeCell` have our `SharedReadOnly` on top. From 224dff4e15f1195c1cdbd85d9080e13b14a151e2 Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Sun, 30 Oct 2022 21:39:16 -0700 Subject: [PATCH 12/38] add acquire when init once is already complete --- src/tools/miri/src/concurrency/init_once.rs | 35 +++++++++-------- src/tools/miri/src/shims/windows/sync.rs | 6 ++- .../pass/concurrency/windows_init_once.rs | 38 +++++++++++++++++++ 3 files changed, 59 insertions(+), 20 deletions(-) diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index 791931901e2a9..b1443662e2b7f 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -141,18 +141,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Wake up everyone. // need to take the queue to avoid having `this` be borrowed multiple times for waiter in std::mem::take(&mut init_once.waiters) { - // End of the wait happens-before woken-up thread. - if let Some(data_race) = &this.machine.data_race { - data_race.validate_lock_acquire( - &this.machine.threads.sync.init_onces[id].data_race, - waiter.thread, - ); - } - this.unblock_thread(waiter.thread); // Call callback, with the woken-up thread as `current`. this.set_active_thread(waiter.thread); + this.init_once_acquire(id); waiter.callback.call(this)?; this.set_active_thread(current_thread); } @@ -172,26 +165,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ); // Each complete happens-before the end of the wait - // FIXME: should this really induce synchronization? If we think of it as a lock, then yes, - // but the docs don't talk about such details. if let Some(data_race) = &this.machine.data_race { data_race.validate_lock_release(&mut init_once.data_race, current_thread); } // Wake up one waiting thread, so they can go ahead and try to init this. if let Some(waiter) = init_once.waiters.pop_front() { - // End of the wait happens-before woken-up thread. - if let Some(data_race) = &this.machine.data_race { - data_race.validate_lock_acquire( - &this.machine.threads.sync.init_onces[id].data_race, - waiter.thread, - ); - } - this.unblock_thread(waiter.thread); // Call callback, with the woken-up thread as `current`. this.set_active_thread(waiter.thread); + this.init_once_acquire(id); waiter.callback.call(this)?; this.set_active_thread(current_thread); } else { @@ -201,4 +185,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ok(()) } + + /// Synchronize with the previous completion or failure of an InitOnce. + /// This is required to prevent data races. + #[inline] + fn init_once_acquire(&mut self, id: InitOnceId) { + let this = self.eval_context_mut(); + let current_thread = this.get_active_thread(); + + if let Some(data_race) = &this.machine.data_race { + data_race.validate_lock_acquire( + &this.machine.threads.sync.init_onces[id].data_race, + current_thread, + ); + } + } } diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 8156ae8af1ef1..f8980e188b45b 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -177,8 +177,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Box::new(Callback { init_once_id: id, pending_place }), ) } - InitOnceStatus::Complete => - this.write_scalar(this.eval_windows("c", "FALSE")?, &pending_place)?, + InitOnceStatus::Complete => { + this.init_once_acquire(id); + this.write_scalar(this.eval_windows("c", "FALSE")?, &pending_place)?; + } } // This always succeeds (even if the thread is blocked, we will succeed if we ever unblock). diff --git a/src/tools/miri/tests/pass/concurrency/windows_init_once.rs b/src/tools/miri/tests/pass/concurrency/windows_init_once.rs index d3c72c3d028cf..6e5129acaf853 100644 --- a/src/tools/miri/tests/pass/concurrency/windows_init_once.rs +++ b/src/tools/miri/tests/pass/concurrency/windows_init_once.rs @@ -131,8 +131,46 @@ fn retry_on_fail() { waiter2.join().unwrap(); } +fn no_data_race_after_complete() { + let mut init_once = null_mut(); + let mut pending = 0; + + unsafe { + assert_eq!(InitOnceBeginInitialize(&mut init_once, 0, &mut pending, null_mut()), TRUE); + assert_eq!(pending, TRUE); + } + + let init_once_ptr = SendPtr(&mut init_once); + + let mut place = 0; + let place_ptr = SendPtr(&mut place); + + let reader = thread::spawn(move || unsafe { + let mut pending = 0; + + assert_eq!(InitOnceBeginInitialize(init_once_ptr.0, 0, &mut pending, null_mut()), TRUE); + assert_eq!(pending, FALSE); + // this should not data race + place_ptr.0.read() + }); + + unsafe { + // this should not data race + place_ptr.0.write(1); + } + + unsafe { + assert_eq!(InitOnceComplete(init_once_ptr.0, 0, null_mut()), TRUE); + } + //println!("complete"); + + // run reader + assert_eq!(reader.join().unwrap(), 1); +} + fn main() { single_thread(); block_until_complete(); retry_on_fail(); + no_data_race_after_complete(); } From a1cd27906c04ea1331c921f0c115135128aadc55 Mon Sep 17 00:00:00 2001 From: Rageking8 Date: Mon, 31 Oct 2022 15:56:48 +0800 Subject: [PATCH 13/38] followup for pr 2640 --- src/tools/miri/cargo-miri/src/phases.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 2c84165a0f0a1..df36041c75ed3 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -528,7 +528,7 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner cmd.args(binary_args); // Make sure we use the build-time working directory for interpreting Miri/rustc arguments. - // But then we need to switch to the run-time one, which we instruct Miri do by setting `MIRI_CWD`. + // But then we need to switch to the run-time one, which we instruct Miri to do by setting `MIRI_CWD`. cmd.current_dir(info.current_dir); cmd.env("MIRI_CWD", env::current_dir().unwrap()); From 95549078d7260dba327bf87cad38d9b87ba10a3a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Nov 2022 08:43:53 +0100 Subject: [PATCH 14/38] fix ./miri bench --- src/tools/miri/miri | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/miri b/src/tools/miri/miri index a0593deb08a37..f0986bfb1cdbe 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -79,6 +79,8 @@ shift MIRIDIR=$(python3 -c 'import os, sys; print(os.path.dirname(os.path.realpath(sys.argv[1])))' "$0") # Used for rustc syncs. JOSH_FILTER=":at_commit=75dd959a3a40eb5b4574f8d2e23aa6efbeb33573[:prefix=src/tools/miri]:/src/tools/miri" +# Needed for `./miri bench`. +TOOLCHAIN=$(cd "$MIRIDIR"; rustup show active-toolchain | head -n 1 | cut -d ' ' -f 1) ## Early commands, that don't do auto-things and don't want the environment-altering things happening below. case "$COMMAND" in @@ -189,6 +191,8 @@ if [ -z "$MIRI_AUTO_OPS" ]; then # other code has run. if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-toolchain" ] ; then $0 toolchain + # Let's make sure to actually use that toolchain, too. + TOOLCHAIN=miri fi if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-fmt" ] ; then @@ -202,7 +206,6 @@ fi ## Prepare the environment # Determine some toolchain properties -TOOLCHAIN=$(cd "$MIRIDIR"; rustup show active-toolchain | head -n 1 | cut -d ' ' -f 1) TARGET=$(rustc +$TOOLCHAIN --version --verbose | grep "^host:" | cut -d ' ' -f 2) SYSROOT=$(rustc +$TOOLCHAIN --print sysroot) LIBDIR=$SYSROOT/lib/rustlib/$TARGET/lib From fa1b720cfc600ed09dd2a6310ece57291c5bae5b Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Thu, 3 Nov 2022 18:13:53 -0700 Subject: [PATCH 15/38] refactor into private functions --- src/tools/miri/src/concurrency/init_once.rs | 75 ++++++++++++++------- src/tools/miri/src/shims/windows/sync.rs | 2 +- 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index b1443662e2b7f..eb42cdf80abbe 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -3,7 +3,7 @@ use std::num::NonZeroU32; use rustc_index::vec::Idx; -use super::sync::EvalContextExtPriv; +use super::sync::EvalContextExtPriv as _; use super::thread::MachineCallback; use super::vector_clock::VClock; use crate::*; @@ -52,6 +52,43 @@ impl<'mir, 'tcx> VisitTags for InitOnce<'mir, 'tcx> { } } +impl<'mir, 'tcx: 'mir> EvalContextExtPriv<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + /// Synchronize with the previous initialization attempt of an InitOnce. + #[inline] + fn init_once_observe_attempt(&mut self, id: InitOnceId) { + let this = self.eval_context_mut(); + let current_thread = this.get_active_thread(); + + if let Some(data_race) = &this.machine.data_race { + data_race.validate_lock_acquire( + &this.machine.threads.sync.init_onces[id].data_race, + current_thread, + ); + } + } + + #[inline] + fn init_once_wake_waiter( + &mut self, + id: InitOnceId, + waiter: InitOnceWaiter<'mir, 'tcx>, + ) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + let current_thread = this.get_active_thread(); + + this.unblock_thread(waiter.thread); + + // Call callback, with the woken-up thread as `current`. + this.set_active_thread(waiter.thread); + this.init_once_observe_attempt(id); + waiter.callback.call(this)?; + this.set_active_thread(current_thread); + + Ok(()) + } +} + impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn init_once_get_or_create_id( @@ -141,13 +178,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Wake up everyone. // need to take the queue to avoid having `this` be borrowed multiple times for waiter in std::mem::take(&mut init_once.waiters) { - this.unblock_thread(waiter.thread); - - // Call callback, with the woken-up thread as `current`. - this.set_active_thread(waiter.thread); - this.init_once_acquire(id); - waiter.callback.call(this)?; - this.set_active_thread(current_thread); + this.init_once_wake_waiter(id, waiter)?; } Ok(()) @@ -171,13 +202,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Wake up one waiting thread, so they can go ahead and try to init this. if let Some(waiter) = init_once.waiters.pop_front() { - this.unblock_thread(waiter.thread); - - // Call callback, with the woken-up thread as `current`. - this.set_active_thread(waiter.thread); - this.init_once_acquire(id); - waiter.callback.call(this)?; - this.set_active_thread(current_thread); + this.init_once_wake_waiter(id, waiter)?; } else { // Nobody there to take this, so go back to 'uninit' init_once.status = InitOnceStatus::Uninitialized; @@ -186,18 +211,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ok(()) } - /// Synchronize with the previous completion or failure of an InitOnce. - /// This is required to prevent data races. + /// Synchronize with the previous completion of an InitOnce. + /// Must only be called after checking that it is complete. #[inline] - fn init_once_acquire(&mut self, id: InitOnceId) { + fn init_once_observe_completed(&mut self, id: InitOnceId) { let this = self.eval_context_mut(); - let current_thread = this.get_active_thread(); - if let Some(data_race) = &this.machine.data_race { - data_race.validate_lock_acquire( - &this.machine.threads.sync.init_onces[id].data_race, - current_thread, - ); - } + assert_eq!( + this.init_once_status(id), + InitOnceStatus::Complete, + "observing the completion of incomplete init once" + ); + + this.init_once_observe_attempt(id); } } diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index f8980e188b45b..098804626f2f9 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -178,7 +178,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) } InitOnceStatus::Complete => { - this.init_once_acquire(id); + this.init_once_observe_completed(id); this.write_scalar(this.eval_windows("c", "FALSE")?, &pending_place)?; } } From cb6f7a6a84e3e808fa95a1206843a96f8b9ff44d Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Thu, 3 Nov 2022 18:30:04 -0700 Subject: [PATCH 16/38] clarify no_data_race_after_complete test --- src/tools/miri/tests/pass/concurrency/windows_init_once.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/tests/pass/concurrency/windows_init_once.rs b/src/tools/miri/tests/pass/concurrency/windows_init_once.rs index 6e5129acaf853..4eb8837962059 100644 --- a/src/tools/miri/tests/pass/concurrency/windows_init_once.rs +++ b/src/tools/miri/tests/pass/concurrency/windows_init_once.rs @@ -148,6 +148,7 @@ fn no_data_race_after_complete() { let reader = thread::spawn(move || unsafe { let mut pending = 0; + // this doesn't block because reader only executes after `InitOnceComplete` is called assert_eq!(InitOnceBeginInitialize(init_once_ptr.0, 0, &mut pending, null_mut()), TRUE); assert_eq!(pending, FALSE); // this should not data race @@ -162,9 +163,8 @@ fn no_data_race_after_complete() { unsafe { assert_eq!(InitOnceComplete(init_once_ptr.0, 0, null_mut()), TRUE); } - //println!("complete"); - // run reader + // run reader (without preemption, it has not taken a step yet) assert_eq!(reader.join().unwrap(), 1); } From a1d94d43ac876975e15b5069da1f9b7a81e29733 Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Sat, 29 Oct 2022 22:58:34 -0700 Subject: [PATCH 17/38] impl condvars for windows --- src/tools/miri/src/concurrency/sync.rs | 14 +- src/tools/miri/src/shims/unix/sync.rs | 14 +- .../miri/src/shims/windows/foreign_items.rs | 19 +++ src/tools/miri/src/shims/windows/sync.rs | 146 ++++++++++++++++++ src/tools/miri/tests/pass/concurrency/sync.rs | 20 +-- .../tests/pass/concurrency/sync_nopreempt.rs | 1 - .../miri/tests/pass/panic/concurrent-panic.rs | 1 - 7 files changed, 185 insertions(+), 30 deletions(-) diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index e76610e730280..48f9e605276e9 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -121,8 +121,10 @@ declare_id!(CondvarId); struct CondvarWaiter { /// The thread that is waiting on this variable. thread: ThreadId, - /// The mutex on which the thread is waiting. - mutex: MutexId, + /// The mutex or rwlock on which the thread is waiting. + lock: u32, + /// If the lock is shared or exclusive + shared: bool, } /// The conditional variable state. @@ -569,16 +571,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } /// Mark that the thread is waiting on the conditional variable. - fn condvar_wait(&mut self, id: CondvarId, thread: ThreadId, mutex: MutexId) { + fn condvar_wait(&mut self, id: CondvarId, thread: ThreadId, lock: u32, shared: bool) { let this = self.eval_context_mut(); let waiters = &mut this.machine.threads.sync.condvars[id].waiters; assert!(waiters.iter().all(|waiter| waiter.thread != thread), "thread is already waiting"); - waiters.push_back(CondvarWaiter { thread, mutex }); + waiters.push_back(CondvarWaiter { thread, lock, shared }); } /// Wake up some thread (if there is any) sleeping on the conditional /// variable. - fn condvar_signal(&mut self, id: CondvarId) -> Option<(ThreadId, MutexId)> { + fn condvar_signal(&mut self, id: CondvarId) -> Option<(ThreadId, u32, bool)> { let this = self.eval_context_mut(); let current_thread = this.get_active_thread(); let condvar = &mut this.machine.threads.sync.condvars[id]; @@ -592,7 +594,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let Some(data_race) = data_race { data_race.validate_lock_acquire(&condvar.data_race, waiter.thread); } - (waiter.thread, waiter.mutex) + (waiter.thread, waiter.lock, waiter.shared) }) } diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index fcb006920794c..d24e1a56bd527 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -696,8 +696,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn pthread_cond_signal(&mut self, cond_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); let id = this.condvar_get_or_create_id(cond_op, CONDVAR_ID_OFFSET)?; - if let Some((thread, mutex)) = this.condvar_signal(id) { - post_cond_signal(this, thread, mutex)?; + if let Some((thread, mutex, shared)) = this.condvar_signal(id) { + assert!(!shared); + post_cond_signal(this, thread, MutexId::from_u32(mutex))?; } Ok(0) @@ -710,8 +711,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); let id = this.condvar_get_or_create_id(cond_op, CONDVAR_ID_OFFSET)?; - while let Some((thread, mutex)) = this.condvar_signal(id) { - post_cond_signal(this, thread, mutex)?; + while let Some((thread, mutex, shared)) = this.condvar_signal(id) { + assert!(!shared); + post_cond_signal(this, thread, MutexId::from_u32(mutex))?; } Ok(0) @@ -729,7 +731,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let active_thread = this.get_active_thread(); release_cond_mutex_and_block(this, active_thread, mutex_id)?; - this.condvar_wait(id, active_thread, mutex_id); + this.condvar_wait(id, active_thread, mutex_id.to_u32(), false); Ok(0) } @@ -768,7 +770,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { }; release_cond_mutex_and_block(this, active_thread, mutex_id)?; - this.condvar_wait(id, active_thread, mutex_id); + this.condvar_wait(id, active_thread, mutex_id.to_u32(), false); // We return success for now and override it in the timeout callback. this.write_scalar(Scalar::from_i32(0), dest)?; diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index 2a34a3a47bbb5..e16749c986b16 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -273,6 +273,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let result = this.InitOnceComplete(ptr, flags, context)?; this.write_scalar(result, dest)?; } + "SleepConditionVariableSRW" => { + let [condvar, lock, timeout, flags] = + this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; + + let result = this.SleepConditionVariableSRW(condvar, lock, timeout, flags, dest)?; + this.write_scalar(result, dest)?; + } + "WakeConditionVariable" => { + let [condvar] = + this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; + + this.WakeConditionVariable(condvar)?; + } + "WakeAllConditionVariable" => { + let [condvar] = + this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; + + this.WakeAllConditionVariable(condvar)?; + } // Dynamic symbol loading "GetProcAddress" => { diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 098804626f2f9..2eab1794c4f44 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -8,6 +8,38 @@ use crate::*; const SRWLOCK_ID_OFFSET: u64 = 0; const INIT_ONCE_ID_OFFSET: u64 = 0; +const CONDVAR_ID_OFFSET: u64 = 0; + +impl<'mir, 'tcx> EvalContextExtPriv<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + /// Try to reacquire the lock associated with the condition variable after we + /// were signaled. + fn reacquire_cond_lock( + &mut self, + thread: ThreadId, + lock: RwLockId, + shared: bool, + ) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + this.unblock_thread(thread); + + if shared { + if this.rwlock_is_locked(lock) { + this.rwlock_enqueue_and_block_reader(lock, thread); + } else { + this.rwlock_reader_lock(lock, thread); + } + } else { + if this.rwlock_is_write_locked(lock) { + this.rwlock_enqueue_and_block_writer(lock, thread); + } else { + this.rwlock_writer_lock(lock, thread); + } + } + + Ok(()) + } +} impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} #[allow(non_snake_case)] @@ -327,4 +359,118 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ok(()) } + + fn SleepConditionVariableSRW( + &mut self, + condvar_op: &OpTy<'tcx, Provenance>, + lock_op: &OpTy<'tcx, Provenance>, + timeout_op: &OpTy<'tcx, Provenance>, + flags_op: &OpTy<'tcx, Provenance>, + dest: &PlaceTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let condvar_id = this.condvar_get_or_create_id(condvar_op, CONDVAR_ID_OFFSET)?; + let lock_id = this.rwlock_get_or_create_id(lock_op, SRWLOCK_ID_OFFSET)?; + let timeout_ms = this.read_scalar(timeout_op)?.to_u32()?; + let flags = this.read_scalar(flags_op)?.to_u32()?; + + let timeout_time = if timeout_ms == this.eval_windows("c", "INFINITE")?.to_u32()? { + None + } else { + let duration = Duration::from_millis(timeout_ms.into()); + Some(this.machine.clock.now().checked_add(duration).unwrap()) + }; + + let shared_mode = 0x1; // CONDITION_VARIABLE_LOCKMODE_SHARED is not in std + let shared = flags == shared_mode; + + let active_thread = this.get_active_thread(); + + let was_locked = if shared { + this.rwlock_reader_unlock(lock_id, active_thread) + } else { + this.rwlock_writer_unlock(lock_id, active_thread) + }; + + if !was_locked { + throw_ub_format!( + "calling SleepConditionVariableSRW with an SRWLock that is not locked by the current thread" + ); + } + + this.block_thread(active_thread); + this.condvar_wait(condvar_id, active_thread, lock_id.to_u32(), shared); + + if let Some(timeout_time) = timeout_time { + struct Callback<'tcx> { + thread: ThreadId, + condvar_id: CondvarId, + lock_id: RwLockId, + shared: bool, + dest: PlaceTy<'tcx, Provenance>, + } + + impl<'tcx> VisitTags for Callback<'tcx> { + fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { + let Callback { thread: _, condvar_id: _, lock_id: _, shared: _, dest } = self; + dest.visit_tags(visit); + } + } + + impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for Callback<'tcx> { + fn call(&self, this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> { + this.reacquire_cond_lock(self.thread, self.lock_id, self.shared)?; + + this.condvar_remove_waiter(self.condvar_id, self.thread); + + let error_timeout = this.eval_windows("c", "ERROR_TIMEOUT")?; + this.set_last_error(error_timeout)?; + this.write_scalar(this.eval_windows("c", "FALSE")?, &self.dest)?; + Ok(()) + } + } + + this.register_timeout_callback( + active_thread, + Time::Monotonic(timeout_time), + Box::new(Callback { + thread: active_thread, + condvar_id, + lock_id, + shared, + dest: dest.clone(), + }), + ); + } + + this.eval_windows("c", "TRUE") + } + + fn WakeConditionVariable(&mut self, condvar_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + let condvar_id = this.condvar_get_or_create_id(condvar_op, CONDVAR_ID_OFFSET)?; + + if let Some((thread, lock, shared)) = this.condvar_signal(condvar_id) { + this.reacquire_cond_lock(thread, RwLockId::from_u32(lock), shared)?; + this.unregister_timeout_callback_if_exists(thread); + } + + Ok(()) + } + + fn WakeAllConditionVariable( + &mut self, + condvar_op: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + let condvar_id = this.condvar_get_or_create_id(condvar_op, CONDVAR_ID_OFFSET)?; + + while let Some((thread, lock, shared)) = this.condvar_signal(condvar_id) { + this.reacquire_cond_lock(thread, RwLockId::from_u32(lock), shared)?; + this.unregister_timeout_callback_if_exists(thread); + } + + Ok(()) + } } diff --git a/src/tools/miri/tests/pass/concurrency/sync.rs b/src/tools/miri/tests/pass/concurrency/sync.rs index b1518a49fbb1b..19ea6c130bdd8 100644 --- a/src/tools/miri/tests/pass/concurrency/sync.rs +++ b/src/tools/miri/tests/pass/concurrency/sync.rs @@ -230,20 +230,8 @@ fn main() { check_once(); park_timeout(); park_unpark(); - - if !cfg!(windows) { - // ignore-target-windows: Condvars on Windows are not supported yet - check_barriers(); - check_conditional_variables_notify_one(); - check_conditional_variables_timed_wait_timeout(); - check_conditional_variables_timed_wait_notimeout(); - } else { - // We need to fake the same output... - for _ in 0..10 { - println!("before wait"); - } - for _ in 0..10 { - println!("after wait"); - } - } + check_barriers(); + check_conditional_variables_notify_one(); + check_conditional_variables_timed_wait_timeout(); + check_conditional_variables_timed_wait_notimeout(); } diff --git a/src/tools/miri/tests/pass/concurrency/sync_nopreempt.rs b/src/tools/miri/tests/pass/concurrency/sync_nopreempt.rs index 55206f4bfc526..c6cff038f81e0 100644 --- a/src/tools/miri/tests/pass/concurrency/sync_nopreempt.rs +++ b/src/tools/miri/tests/pass/concurrency/sync_nopreempt.rs @@ -1,4 +1,3 @@ -//@ignore-target-windows: Condvars on Windows are not supported yet. // We are making scheduler assumptions here. //@compile-flags: -Zmiri-strict-provenance -Zmiri-preemption-rate=0 diff --git a/src/tools/miri/tests/pass/panic/concurrent-panic.rs b/src/tools/miri/tests/pass/panic/concurrent-panic.rs index 342269c6acbe3..776bc2057f350 100644 --- a/src/tools/miri/tests/pass/panic/concurrent-panic.rs +++ b/src/tools/miri/tests/pass/panic/concurrent-panic.rs @@ -1,4 +1,3 @@ -//@ignore-target-windows: Condvars on Windows are not supported yet. // We are making scheduler assumptions here. //@compile-flags: -Zmiri-preemption-rate=0 From a2f7e8497e12e6d79d7963729ef6746b912a32eb Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Sun, 30 Oct 2022 14:56:49 -0700 Subject: [PATCH 18/38] use enum for condvar locks --- src/tools/miri/src/concurrency/sync.rs | 24 +++++--- src/tools/miri/src/shims/unix/sync.rs | 23 +++++--- src/tools/miri/src/shims/windows/sync.rs | 73 ++++++++++++++---------- 3 files changed, 76 insertions(+), 44 deletions(-) diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 48f9e605276e9..ba5ae852c5a96 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -116,15 +116,25 @@ struct RwLock { declare_id!(CondvarId); +#[derive(Debug, Copy, Clone)] +pub enum RwLockMode { + Read, + Write, +} + +#[derive(Debug)] +pub enum CondvarLock { + Mutex(MutexId), + RwLock { id: RwLockId, mode: RwLockMode }, +} + /// A thread waiting on a conditional variable. #[derive(Debug)] struct CondvarWaiter { /// The thread that is waiting on this variable. thread: ThreadId, /// The mutex or rwlock on which the thread is waiting. - lock: u32, - /// If the lock is shared or exclusive - shared: bool, + lock: CondvarLock, } /// The conditional variable state. @@ -571,16 +581,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } /// Mark that the thread is waiting on the conditional variable. - fn condvar_wait(&mut self, id: CondvarId, thread: ThreadId, lock: u32, shared: bool) { + fn condvar_wait(&mut self, id: CondvarId, thread: ThreadId, lock: CondvarLock) { let this = self.eval_context_mut(); let waiters = &mut this.machine.threads.sync.condvars[id].waiters; assert!(waiters.iter().all(|waiter| waiter.thread != thread), "thread is already waiting"); - waiters.push_back(CondvarWaiter { thread, lock, shared }); + waiters.push_back(CondvarWaiter { thread, lock }); } /// Wake up some thread (if there is any) sleeping on the conditional /// variable. - fn condvar_signal(&mut self, id: CondvarId) -> Option<(ThreadId, u32, bool)> { + fn condvar_signal(&mut self, id: CondvarId) -> Option<(ThreadId, CondvarLock)> { let this = self.eval_context_mut(); let current_thread = this.get_active_thread(); let condvar = &mut this.machine.threads.sync.condvars[id]; @@ -594,7 +604,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let Some(data_race) = data_race { data_race.validate_lock_acquire(&condvar.data_race, waiter.thread); } - (waiter.thread, waiter.lock, waiter.shared) + (waiter.thread, waiter.lock) }) } diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index d24e1a56bd527..a7275646847e2 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -3,6 +3,7 @@ use std::time::SystemTime; use rustc_hir::LangItem; use rustc_middle::ty::{layout::TyAndLayout, query::TyCtxtAt, Ty}; +use crate::concurrency::sync::CondvarLock; use crate::concurrency::thread::{MachineCallback, Time}; use crate::*; @@ -696,9 +697,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn pthread_cond_signal(&mut self, cond_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); let id = this.condvar_get_or_create_id(cond_op, CONDVAR_ID_OFFSET)?; - if let Some((thread, mutex, shared)) = this.condvar_signal(id) { - assert!(!shared); - post_cond_signal(this, thread, MutexId::from_u32(mutex))?; + if let Some((thread, lock)) = this.condvar_signal(id) { + if let CondvarLock::Mutex(mutex) = lock { + post_cond_signal(this, thread, mutex)?; + } else { + panic!("condvar should not have an rwlock on unix"); + } } Ok(0) @@ -711,9 +715,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); let id = this.condvar_get_or_create_id(cond_op, CONDVAR_ID_OFFSET)?; - while let Some((thread, mutex, shared)) = this.condvar_signal(id) { - assert!(!shared); - post_cond_signal(this, thread, MutexId::from_u32(mutex))?; + while let Some((thread, lock)) = this.condvar_signal(id) { + if let CondvarLock::Mutex(mutex) = lock { + post_cond_signal(this, thread, mutex)?; + } else { + panic!("condvar should not have an rwlock on unix"); + } } Ok(0) @@ -731,7 +738,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let active_thread = this.get_active_thread(); release_cond_mutex_and_block(this, active_thread, mutex_id)?; - this.condvar_wait(id, active_thread, mutex_id.to_u32(), false); + this.condvar_wait(id, active_thread, CondvarLock::Mutex(mutex_id)); Ok(0) } @@ -770,7 +777,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { }; release_cond_mutex_and_block(this, active_thread, mutex_id)?; - this.condvar_wait(id, active_thread, mutex_id.to_u32(), false); + this.condvar_wait(id, active_thread, CondvarLock::Mutex(mutex_id)); // We return success for now and override it in the timeout callback. this.write_scalar(Scalar::from_i32(0), dest)?; diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 2eab1794c4f44..a34d142f4a5ef 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -3,6 +3,7 @@ use std::time::Duration; use rustc_target::abi::Size; use crate::concurrency::init_once::InitOnceStatus; +use crate::concurrency::sync::{CondvarLock, RwLockMode}; use crate::concurrency::thread::MachineCallback; use crate::*; @@ -18,23 +19,24 @@ pub trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tc &mut self, thread: ThreadId, lock: RwLockId, - shared: bool, + mode: RwLockMode, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); this.unblock_thread(thread); - if shared { - if this.rwlock_is_locked(lock) { - this.rwlock_enqueue_and_block_reader(lock, thread); - } else { - this.rwlock_reader_lock(lock, thread); - } - } else { - if this.rwlock_is_write_locked(lock) { - this.rwlock_enqueue_and_block_writer(lock, thread); - } else { - this.rwlock_writer_lock(lock, thread); - } + match mode { + RwLockMode::Read => + if this.rwlock_is_locked(lock) { + this.rwlock_enqueue_and_block_reader(lock, thread); + } else { + this.rwlock_reader_lock(lock, thread); + }, + RwLockMode::Write => + if this.rwlock_is_write_locked(lock) { + this.rwlock_enqueue_and_block_writer(lock, thread); + } else { + this.rwlock_writer_lock(lock, thread); + }, } Ok(()) @@ -383,14 +385,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { }; let shared_mode = 0x1; // CONDITION_VARIABLE_LOCKMODE_SHARED is not in std - let shared = flags == shared_mode; + let mode = if flags == 0 { + RwLockMode::Write + } else if flags == shared_mode { + RwLockMode::Read + } else { + throw_unsup_format!("unsupported `Flags` {flags} in `SleepConditionVariableSRW`"); + }; let active_thread = this.get_active_thread(); - let was_locked = if shared { - this.rwlock_reader_unlock(lock_id, active_thread) - } else { - this.rwlock_writer_unlock(lock_id, active_thread) + let was_locked = match mode { + RwLockMode::Read => this.rwlock_reader_unlock(lock_id, active_thread), + RwLockMode::Write => this.rwlock_writer_unlock(lock_id, active_thread), }; if !was_locked { @@ -400,27 +407,27 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } this.block_thread(active_thread); - this.condvar_wait(condvar_id, active_thread, lock_id.to_u32(), shared); + this.condvar_wait(condvar_id, active_thread, CondvarLock::RwLock { id: lock_id, mode }); if let Some(timeout_time) = timeout_time { struct Callback<'tcx> { thread: ThreadId, condvar_id: CondvarId, lock_id: RwLockId, - shared: bool, + mode: RwLockMode, dest: PlaceTy<'tcx, Provenance>, } impl<'tcx> VisitTags for Callback<'tcx> { fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { - let Callback { thread: _, condvar_id: _, lock_id: _, shared: _, dest } = self; + let Callback { thread: _, condvar_id: _, lock_id: _, mode: _, dest } = self; dest.visit_tags(visit); } } impl<'mir, 'tcx: 'mir> MachineCallback<'mir, 'tcx> for Callback<'tcx> { fn call(&self, this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> { - this.reacquire_cond_lock(self.thread, self.lock_id, self.shared)?; + this.reacquire_cond_lock(self.thread, self.lock_id, self.mode)?; this.condvar_remove_waiter(self.condvar_id, self.thread); @@ -438,7 +445,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { thread: active_thread, condvar_id, lock_id, - shared, + mode, dest: dest.clone(), }), ); @@ -451,9 +458,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); let condvar_id = this.condvar_get_or_create_id(condvar_op, CONDVAR_ID_OFFSET)?; - if let Some((thread, lock, shared)) = this.condvar_signal(condvar_id) { - this.reacquire_cond_lock(thread, RwLockId::from_u32(lock), shared)?; - this.unregister_timeout_callback_if_exists(thread); + if let Some((thread, lock)) = this.condvar_signal(condvar_id) { + if let CondvarLock::RwLock { id, mode } = lock { + this.reacquire_cond_lock(thread, id, mode)?; + this.unregister_timeout_callback_if_exists(thread); + } else { + panic!("mutexes should not exist on windows"); + } } Ok(()) @@ -466,9 +477,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); let condvar_id = this.condvar_get_or_create_id(condvar_op, CONDVAR_ID_OFFSET)?; - while let Some((thread, lock, shared)) = this.condvar_signal(condvar_id) { - this.reacquire_cond_lock(thread, RwLockId::from_u32(lock), shared)?; - this.unregister_timeout_callback_if_exists(thread); + while let Some((thread, lock)) = this.condvar_signal(condvar_id) { + if let CondvarLock::RwLock { id, mode } = lock { + this.reacquire_cond_lock(thread, id, mode)?; + this.unregister_timeout_callback_if_exists(thread); + } else { + panic!("mutexes should not exist on windows"); + } } Ok(()) From 2eb07a05a3fa1e604f4243ddd5659f294d6b69b9 Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Sun, 30 Oct 2022 19:14:45 -0700 Subject: [PATCH 19/38] fix shared behavior and add tests --- src/tools/miri/src/shims/windows/sync.rs | 6 +- .../concurrency/windows_condvar_shared.rs | 227 ++++++++++++++++++ .../concurrency/windows_condvar_shared.stdout | 60 +++++ 3 files changed, 290 insertions(+), 3 deletions(-) create mode 100644 src/tools/miri/tests/pass/concurrency/windows_condvar_shared.rs create mode 100644 src/tools/miri/tests/pass/concurrency/windows_condvar_shared.stdout diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index a34d142f4a5ef..8f414d98dba5f 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -12,7 +12,7 @@ const INIT_ONCE_ID_OFFSET: u64 = 0; const CONDVAR_ID_OFFSET: u64 = 0; impl<'mir, 'tcx> EvalContextExtPriv<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} -pub trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { +trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Try to reacquire the lock associated with the condition variable after we /// were signaled. fn reacquire_cond_lock( @@ -26,13 +26,13 @@ pub trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tc match mode { RwLockMode::Read => - if this.rwlock_is_locked(lock) { + if this.rwlock_is_write_locked(lock) { this.rwlock_enqueue_and_block_reader(lock, thread); } else { this.rwlock_reader_lock(lock, thread); }, RwLockMode::Write => - if this.rwlock_is_write_locked(lock) { + if this.rwlock_is_locked(lock) { this.rwlock_enqueue_and_block_writer(lock, thread); } else { this.rwlock_writer_lock(lock, thread); diff --git a/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.rs b/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.rs new file mode 100644 index 0000000000000..d89320bfe5971 --- /dev/null +++ b/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.rs @@ -0,0 +1,227 @@ +//@only-target-windows: Uses win32 api functions +// We are making scheduler assumptions here. +//@compile-flags: -Zmiri-preemption-rate=0 + +use std::ffi::c_void; +use std::ptr::null_mut; +use std::thread; + +#[derive(Copy, Clone)] +struct SendPtr(*mut T); + +unsafe impl Send for SendPtr {} + +extern "system" { + fn SleepConditionVariableSRW( + condvar: *mut *mut c_void, + lock: *mut *mut c_void, + timeout: u32, + flags: u32, + ) -> i32; + fn WakeAllConditionVariable(condvar: *mut *mut c_void); + + fn AcquireSRWLockExclusive(lock: *mut *mut c_void); + fn AcquireSRWLockShared(lock: *mut *mut c_void); + fn ReleaseSRWLockExclusive(lock: *mut *mut c_void); + fn ReleaseSRWLockShared(lock: *mut *mut c_void); +} + +const CONDITION_VARIABLE_LOCKMODE_SHARED: u32 = 1; +const INFINITE: u32 = u32::MAX; + +/// threads should be able to reacquire the lock while it is locked by multiple other threads in shared mode +fn all_shared() { + println!("all_shared"); + + let mut lock = null_mut(); + let mut condvar = null_mut(); + + let lock_ptr = SendPtr(&mut lock); + let condvar_ptr = SendPtr(&mut condvar); + + let mut handles = Vec::with_capacity(10); + + // waiters + for i in 0..5 { + handles.push(thread::spawn(move || { + unsafe { + AcquireSRWLockShared(lock_ptr.0); + } + println!("exclusive waiter {i} locked"); + + let r = unsafe { + SleepConditionVariableSRW( + condvar_ptr.0, + lock_ptr.0, + INFINITE, + CONDITION_VARIABLE_LOCKMODE_SHARED, + ) + }; + assert_ne!(r, 0); + + println!("exclusive waiter {i} reacquired lock"); + + // unlocking is unnecessary because the lock is never used again + })); + } + + // ensures each waiter is waiting by this point + thread::yield_now(); + + // readers + for i in 0..5 { + handles.push(thread::spawn(move || { + unsafe { + AcquireSRWLockShared(lock_ptr.0); + } + println!("reader {i} locked"); + + // switch to next reader or main thread + thread::yield_now(); + + unsafe { + ReleaseSRWLockShared(lock_ptr.0); + } + println!("reader {i} unlocked"); + })); + } + + // ensures each reader has acquired the lock + thread::yield_now(); + + unsafe { + WakeAllConditionVariable(condvar_ptr.0); + } + + for handle in handles { + handle.join().unwrap(); + } +} + +// reacquiring a lock should wait until the lock is not exclusively locked +fn shared_sleep_and_exclusive_lock() { + println!("shared_sleep_and_exclusive_lock"); + + let mut lock = null_mut(); + let mut condvar = null_mut(); + + let lock_ptr = SendPtr(&mut lock); + let condvar_ptr = SendPtr(&mut condvar); + + let mut waiters = Vec::with_capacity(5); + for i in 0..5 { + waiters.push(thread::spawn(move || { + unsafe { + AcquireSRWLockShared(lock_ptr.0); + } + println!("shared waiter {i} locked"); + + let r = unsafe { + SleepConditionVariableSRW( + condvar_ptr.0, + lock_ptr.0, + INFINITE, + CONDITION_VARIABLE_LOCKMODE_SHARED, + ) + }; + assert_ne!(r, 0); + + println!("shared waiter {i} reacquired lock"); + + // unlocking is unnecessary because the lock is never used again + })); + } + + // ensures each waiter is waiting by this point + thread::yield_now(); + + unsafe { + AcquireSRWLockExclusive(lock_ptr.0); + } + println!("main locked"); + + unsafe { + WakeAllConditionVariable(condvar_ptr.0); + } + + // waiters are now waiting for the lock to be unlocked + thread::yield_now(); + + unsafe { + ReleaseSRWLockExclusive(lock_ptr.0); + } + println!("main unlocked"); + + for handle in waiters { + handle.join().unwrap(); + } +} + +// threads reacquiring locks should wait for all locks to be released first +fn exclusive_sleep_and_shared_lock() { + println!("exclusive_sleep_and_shared_lock"); + + let mut lock = null_mut(); + let mut condvar = null_mut(); + + let lock_ptr = SendPtr(&mut lock); + let condvar_ptr = SendPtr(&mut condvar); + + let mut handles = Vec::with_capacity(10); + for i in 0..5 { + handles.push(thread::spawn(move || { + unsafe { + AcquireSRWLockExclusive(lock_ptr.0); + } + + println!("exclusive waiter {i} locked"); + + let r = unsafe { SleepConditionVariableSRW(condvar_ptr.0, lock_ptr.0, INFINITE, 0) }; + assert_ne!(r, 0); + + println!("exclusive waiter {i} reacquired lock"); + + // switch to next waiter or main thread + thread::yield_now(); + + unsafe { + ReleaseSRWLockExclusive(lock_ptr.0); + } + println!("exclusive waiter {i} unlocked"); + })); + } + + for i in 0..5 { + handles.push(thread::spawn(move || { + unsafe { + AcquireSRWLockShared(lock_ptr.0); + } + println!("reader {i} locked"); + + // switch to next reader or main thread + thread::yield_now(); + + unsafe { + ReleaseSRWLockShared(lock_ptr.0); + } + println!("reader {i} unlocked"); + })); + } + + // ensures each reader has acquired the lock + thread::yield_now(); + + unsafe { + WakeAllConditionVariable(condvar_ptr.0); + } + + for handle in handles { + handle.join().unwrap(); + } +} + +fn main() { + all_shared(); + shared_sleep_and_exclusive_lock(); + exclusive_sleep_and_shared_lock(); +} diff --git a/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.stdout b/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.stdout new file mode 100644 index 0000000000000..918b54668f201 --- /dev/null +++ b/src/tools/miri/tests/pass/concurrency/windows_condvar_shared.stdout @@ -0,0 +1,60 @@ +all_shared +exclusive waiter 0 locked +exclusive waiter 1 locked +exclusive waiter 2 locked +exclusive waiter 3 locked +exclusive waiter 4 locked +reader 0 locked +reader 1 locked +reader 2 locked +reader 3 locked +reader 4 locked +exclusive waiter 0 reacquired lock +exclusive waiter 1 reacquired lock +exclusive waiter 2 reacquired lock +exclusive waiter 3 reacquired lock +exclusive waiter 4 reacquired lock +reader 0 unlocked +reader 1 unlocked +reader 2 unlocked +reader 3 unlocked +reader 4 unlocked +shared_sleep_and_exclusive_lock +shared waiter 0 locked +shared waiter 1 locked +shared waiter 2 locked +shared waiter 3 locked +shared waiter 4 locked +main locked +main unlocked +shared waiter 0 reacquired lock +shared waiter 1 reacquired lock +shared waiter 2 reacquired lock +shared waiter 3 reacquired lock +shared waiter 4 reacquired lock +exclusive_sleep_and_shared_lock +exclusive waiter 0 locked +exclusive waiter 1 locked +exclusive waiter 2 locked +exclusive waiter 3 locked +exclusive waiter 4 locked +reader 0 locked +reader 1 locked +reader 2 locked +reader 3 locked +reader 4 locked +reader 0 unlocked +reader 1 unlocked +reader 2 unlocked +reader 3 unlocked +reader 4 unlocked +exclusive waiter 0 reacquired lock +exclusive waiter 0 unlocked +exclusive waiter 1 reacquired lock +exclusive waiter 1 unlocked +exclusive waiter 2 reacquired lock +exclusive waiter 2 unlocked +exclusive waiter 3 reacquired lock +exclusive waiter 3 unlocked +exclusive waiter 4 reacquired lock +exclusive waiter 4 unlocked From 8e0cac18cd2951e2679ea55e15242d04e2d410c9 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 7 Nov 2022 11:13:01 -0700 Subject: [PATCH 20/38] rustdoc: refactor `notable_traits_decl` to just act on the type directly --- src/librustdoc/html/render/mod.rs | 133 +++++++++++------------ src/librustdoc/html/render/print_item.rs | 7 +- 2 files changed, 72 insertions(+), 68 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 3a041ae15d618..881f107925315 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -861,7 +861,11 @@ fn assoc_method( name = name, generics = g.print(cx), decl = d.full_print(header_len, indent, cx), - notable_traits = notable_traits_decl(d, cx), + notable_traits = d + .output + .as_return() + .and_then(|output| notable_traits_decl(output, cx)) + .unwrap_or_default(), where_clause = print_where_clause(g, cx, indent, end_newline), ) } @@ -1273,71 +1277,64 @@ fn should_render_item(item: &clean::Item, deref_mut_: bool, tcx: TyCtxt<'_>) -> } } -fn notable_traits_decl(decl: &clean::FnDecl, cx: &Context<'_>) -> String { +fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> Option { let mut out = Buffer::html(); - if let Some((did, ty)) = decl.output.as_return().and_then(|t| Some((t.def_id(cx.cache())?, t))) + let did = ty.def_id(cx.cache())?; + + // Box has pass-through impls for Read, Write, Iterator, and Future when the + // boxed type implements one of those. We don't want to treat every Box return + // as being notably an Iterator (etc), though, so we exempt it. Pin has the same + // issue, with a pass-through impl for Future. + if Some(did) == cx.tcx().lang_items().owned_box() + || Some(did) == cx.tcx().lang_items().pin_type() { - // Box has pass-through impls for Read, Write, Iterator, and Future when the - // boxed type implements one of those. We don't want to treat every Box return - // as being notably an Iterator (etc), though, so we exempt it. Pin has the same - // issue, with a pass-through impl for Future. - if Some(did) == cx.tcx().lang_items().owned_box() - || Some(did) == cx.tcx().lang_items().pin_type() - { - return "".to_string(); - } - if let Some(impls) = cx.cache().impls.get(&did) { - for i in impls { - let impl_ = i.inner_impl(); - if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) - { - // Two different types might have the same did, - // without actually being the same. - continue; - } - if let Some(trait_) = &impl_.trait_ { - let trait_did = trait_.def_id(); - - if cx - .cache() - .traits - .get(&trait_did) - .map_or(false, |t| t.is_notable_trait(cx.tcx())) - { - if out.is_empty() { - write!( - &mut out, - "Notable traits for {}\ - ", - impl_.for_.print(cx) - ); - } + return None; + } + if let Some(impls) = cx.cache().impls.get(&did) { + for i in impls { + let impl_ = i.inner_impl(); + if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) { + // Two different types might have the same did, + // without actually being the same. + continue; + } + if let Some(trait_) = &impl_.trait_ { + let trait_did = trait_.def_id(); - //use the "where" class here to make it small + if cx.cache().traits.get(&trait_did).map_or(false, |t| t.is_notable_trait(cx.tcx())) + { + if out.is_empty() { write!( &mut out, - "{}", - impl_.print(false, cx) + "Notable traits for {}\ + ", + impl_.for_.print(cx) ); - for it in &impl_.items { - if let clean::AssocTypeItem(ref tydef, ref _bounds) = *it.kind { - out.push_str(" "); - let empty_set = FxHashSet::default(); - let src_link = - AssocItemLink::GotoSource(trait_did.into(), &empty_set); - assoc_type( - &mut out, - it, - &tydef.generics, - &[], // intentionally leaving out bounds - Some(&tydef.type_), - src_link, - 0, - cx, - ); - out.push_str(";"); - } + } + + //use the "where" class here to make it small + write!( + &mut out, + "{}", + impl_.print(false, cx) + ); + for it in &impl_.items { + if let clean::AssocTypeItem(ref tydef, ref _bounds) = *it.kind { + out.push_str(" "); + let empty_set = FxHashSet::default(); + let src_link = AssocItemLink::GotoSource(trait_did.into(), &empty_set); + assoc_type( + &mut out, + it, + &tydef.generics, + &[], // intentionally leaving out bounds + Some(&tydef.type_), + src_link, + 0, + cx, + ); + out.push_str(";"); } } } @@ -1345,16 +1342,18 @@ fn notable_traits_decl(decl: &clean::FnDecl, cx: &Context<'_>) -> String { } } - if !out.is_empty() { - out.insert_str( - 0, - "ⓘ\ - ", - ); - out.push_str(""); + if out.is_empty() { + return None; } - out.into_inner() + out.insert_str( + 0, + "ⓘ\ + ", + ); + out.push_str(""); + + Some(out.into_inner()) } #[derive(Clone, Copy, Debug)] diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index ce4fc4d68fac2..e6abd23eb951b 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -533,7 +533,12 @@ fn item_function(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, f: &cle generics = f.generics.print(cx), where_clause = print_where_clause(&f.generics, cx, 0, Ending::Newline), decl = f.decl.full_print(header_len, 0, cx), - notable_traits = notable_traits_decl(&f.decl, cx), + notable_traits = f + .decl + .output + .as_return() + .and_then(|output| notable_traits_decl(output, cx)) + .unwrap_or_default(), ); }); }); From 303653ef65a337b21226a52546615936225fb5af Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 7 Nov 2022 15:53:30 -0700 Subject: [PATCH 21/38] rustdoc: use javascript to layout notable traits popups Fixes #102576 --- src/librustdoc/html/format.rs | 4 - src/librustdoc/html/render/context.rs | 7 +- src/librustdoc/html/render/mod.rs | 145 ++++++++++++------ src/librustdoc/html/render/print_item.rs | 29 ++-- src/librustdoc/html/static/css/noscript.css | 6 + src/librustdoc/html/static/css/rustdoc.css | 23 +-- src/librustdoc/html/static/js/main.js | 85 +++++++++- src/test/rustdoc-gui/notable-trait.goml | 29 ++-- ...c-notable_trait-slice.bare_fn_matches.html | 1 + src/test/rustdoc/doc-notable_trait-slice.rs | 4 +- .../rustdoc/doc-notable_trait.bare-fn.html | 1 + src/test/rustdoc/doc-notable_trait.rs | 10 +- .../doc-notable_trait.some-struct-new.html | 1 + .../rustdoc/doc-notable_trait.wrap-me.html | 1 + .../spotlight-from-dependency.odd.html | 1 + src/test/rustdoc/spotlight-from-dependency.rs | 3 +- 16 files changed, 260 insertions(+), 90 deletions(-) create mode 100644 src/test/rustdoc/doc-notable_trait-slice.bare_fn_matches.html create mode 100644 src/test/rustdoc/doc-notable_trait.bare-fn.html create mode 100644 src/test/rustdoc/doc-notable_trait.some-struct-new.html create mode 100644 src/test/rustdoc/doc-notable_trait.wrap-me.html create mode 100644 src/test/rustdoc/spotlight-from-dependency.odd.html diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index a5c3d35b1b594..39e2a90222670 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -107,10 +107,6 @@ impl Buffer { self.buffer } - pub(crate) fn insert_str(&mut self, idx: usize, s: &str) { - self.buffer.insert_str(idx, s); - } - pub(crate) fn push_str(&mut self, s: &str) { self.buffer.push_str(s); } diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 51843a505f709..73690c86f4f72 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -69,11 +69,13 @@ pub(crate) struct Context<'tcx> { /// the source files are present in the html rendering, then this will be /// `true`. pub(crate) include_sources: bool, + /// Collection of all types with notable traits referenced in the current module. + pub(crate) types_with_notable_traits: FxHashSet, } // `Context` is cloned a lot, so we don't want the size to grow unexpectedly. #[cfg(all(not(windows), target_arch = "x86_64", target_pointer_width = "64"))] -rustc_data_structures::static_assert_size!(Context<'_>, 128); +rustc_data_structures::static_assert_size!(Context<'_>, 160); /// Shared mutable state used in [`Context`] and elsewhere. pub(crate) struct SharedContext<'tcx> { @@ -532,6 +534,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { deref_id_map: FxHashMap::default(), shared: Rc::new(scx), include_sources, + types_with_notable_traits: FxHashSet::default(), }; if emit_crate { @@ -560,6 +563,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { id_map: IdMap::new(), shared: Rc::clone(&self.shared), include_sources: self.include_sources, + types_with_notable_traits: FxHashSet::default(), } } @@ -803,6 +807,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { } } } + Ok(()) } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 881f107925315..1c13e2bf67781 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -59,7 +59,7 @@ use rustc_span::{ symbol::{sym, Symbol}, BytePos, FileName, RealFileName, }; -use serde::ser::SerializeSeq; +use serde::ser::{SerializeMap, SerializeSeq}; use serde::{Serialize, Serializer}; use crate::clean::{self, ItemId, RenderedLink, SelfTy}; @@ -803,7 +803,7 @@ fn assoc_method( d: &clean::FnDecl, link: AssocItemLink<'_>, parent: ItemType, - cx: &Context<'_>, + cx: &mut Context<'_>, render_mode: RenderMode, ) { let tcx = cx.tcx(); @@ -836,6 +836,8 @@ fn assoc_method( + name.as_str().len() + generics_len; + let notable_traits = d.output.as_return().and_then(|output| notable_traits_button(output, cx)); + let (indent, indent_str, end_newline) = if parent == ItemType::Trait { header_len += 4; let indent_str = " "; @@ -861,13 +863,9 @@ fn assoc_method( name = name, generics = g.print(cx), decl = d.full_print(header_len, indent, cx), - notable_traits = d - .output - .as_return() - .and_then(|output| notable_traits_decl(output, cx)) - .unwrap_or_default(), + notable_traits = notable_traits.unwrap_or_default(), where_clause = print_where_clause(g, cx, indent, end_newline), - ) + ); } /// Writes a span containing the versions at which an item became stable and/or const-stable. For @@ -967,7 +965,7 @@ fn render_assoc_item( item: &clean::Item, link: AssocItemLink<'_>, parent: ItemType, - cx: &Context<'_>, + cx: &mut Context<'_>, render_mode: RenderMode, ) { match &*item.kind { @@ -1277,8 +1275,8 @@ fn should_render_item(item: &clean::Item, deref_mut_: bool, tcx: TyCtxt<'_>) -> } } -fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> Option { - let mut out = Buffer::html(); +pub(crate) fn notable_traits_button(ty: &clean::Type, cx: &mut Context<'_>) -> Option { + let mut has_notable_trait = false; let did = ty.def_id(cx.cache())?; @@ -1291,6 +1289,7 @@ fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> Option { { return None; } + if let Some(impls) = cx.cache().impls.get(&did) { for i in impls { let impl_ = i.inner_impl(); @@ -1304,56 +1303,106 @@ fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> Option { if cx.cache().traits.get(&trait_did).map_or(false, |t| t.is_notable_trait(cx.tcx())) { - if out.is_empty() { - write!( - &mut out, - "Notable traits for {}\ - ", - impl_.for_.print(cx) - ); - } + has_notable_trait = true; + } + } + } + } + + if has_notable_trait { + cx.types_with_notable_traits.insert(ty.clone()); + Some(format!( + "\ + \ + ", + ty = ty.print(cx), + )) + } else { + None + } +} + +fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> (String, String) { + let mut out = Buffer::html(); + + let did = ty.def_id(cx.cache()).expect("notable_traits_button already checked this"); - //use the "where" class here to make it small + let impls = cx.cache().impls.get(&did).expect("notable_traits_button already checked this"); + + for i in impls { + let impl_ = i.inner_impl(); + if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) { + // Two different types might have the same did, + // without actually being the same. + continue; + } + if let Some(trait_) = &impl_.trait_ { + let trait_did = trait_.def_id(); + + if cx.cache().traits.get(&trait_did).map_or(false, |t| t.is_notable_trait(cx.tcx())) { + if out.is_empty() { write!( &mut out, - "{}", - impl_.print(false, cx) + "

Notable traits for {}

\ +
",
+                        impl_.for_.print(cx)
                     );
-                    for it in &impl_.items {
-                        if let clean::AssocTypeItem(ref tydef, ref _bounds) = *it.kind {
-                            out.push_str("    ");
-                            let empty_set = FxHashSet::default();
-                            let src_link = AssocItemLink::GotoSource(trait_did.into(), &empty_set);
-                            assoc_type(
-                                &mut out,
-                                it,
-                                &tydef.generics,
-                                &[], // intentionally leaving out bounds
-                                Some(&tydef.type_),
-                                src_link,
-                                0,
-                                cx,
-                            );
-                            out.push_str(";");
-                        }
+                }
+
+                //use the "where" class here to make it small
+                write!(
+                    &mut out,
+                    "{}",
+                    impl_.print(false, cx)
+                );
+                for it in &impl_.items {
+                    if let clean::AssocTypeItem(ref tydef, ref _bounds) = *it.kind {
+                        out.push_str("    ");
+                        let empty_set = FxHashSet::default();
+                        let src_link = AssocItemLink::GotoSource(trait_did.into(), &empty_set);
+                        assoc_type(
+                            &mut out,
+                            it,
+                            &tydef.generics,
+                            &[], // intentionally leaving out bounds
+                            Some(&tydef.type_),
+                            src_link,
+                            0,
+                            cx,
+                        );
+                        out.push_str(";");
                     }
                 }
             }
         }
     }
-
     if out.is_empty() {
-        return None;
+        write!(&mut out, "
",); } - out.insert_str( - 0, - "ⓘ\ - ", - ); - out.push_str("
"); + (format!("{:#}", ty.print(cx)), out.into_inner()) +} - Some(out.into_inner()) +pub(crate) fn notable_traits_json<'a>( + tys: impl Iterator, + cx: &Context<'_>, +) -> String { + let mp: Vec<(String, String)> = tys.map(|ty| notable_traits_decl(ty, cx)).collect(); + struct NotableTraitsMap(Vec<(String, String)>); + impl Serialize for NotableTraitsMap { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let mut map = serializer.serialize_map(Some(self.0.len()))?; + for item in &self.0 { + map.serialize_entry(&item.0, &item.1)?; + } + map.end() + } + } + serde_json::to_string(&NotableTraitsMap(mp)) + .expect("serialize (string, string) -> json object cannot fail") } #[derive(Clone, Copy, Debug)] diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index e6abd23eb951b..ac11a860a4f0b 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -17,9 +17,10 @@ use std::rc::Rc; use super::{ collect_paths_for_type, document, ensure_trailing_slash, get_filtered_impls_for_reference, - item_ty_to_section, notable_traits_decl, render_all_impls, render_assoc_item, - render_assoc_items, render_attributes_in_code, render_attributes_in_pre, render_impl, - render_rightside, render_stability_since_raw, AssocItemLink, Context, ImplRenderingParameters, + item_ty_to_section, notable_traits_button, notable_traits_json, render_all_impls, + render_assoc_item, render_assoc_items, render_attributes_in_code, render_attributes_in_pre, + render_impl, render_rightside, render_stability_since_raw, AssocItemLink, Context, + ImplRenderingParameters, }; use crate::clean; use crate::config::ModuleSorting; @@ -183,6 +184,16 @@ pub(super) fn print_item( unreachable!(); } } + + // Render notable-traits.js used for all methods in this module. + if !cx.types_with_notable_traits.is_empty() { + write!( + buf, + r#""#, + notable_traits_json(cx.types_with_notable_traits.iter(), cx) + ); + cx.types_with_notable_traits.clear(); + } } /// For large structs, enums, unions, etc, determine whether to hide their fields @@ -516,6 +527,9 @@ fn item_function(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, f: &cle + name.as_str().len() + generics_len; + let notable_traits = + f.decl.output.as_return().and_then(|output| notable_traits_button(output, cx)); + wrap_into_item_decl(w, |w| { wrap_item(w, "fn", |w| { render_attributes_in_pre(w, it, ""); @@ -533,16 +547,11 @@ fn item_function(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, f: &cle generics = f.generics.print(cx), where_clause = print_where_clause(&f.generics, cx, 0, Ending::Newline), decl = f.decl.full_print(header_len, 0, cx), - notable_traits = f - .decl - .output - .as_return() - .and_then(|output| notable_traits_decl(output, cx)) - .unwrap_or_default(), + notable_traits = notable_traits.unwrap_or_default(), ); }); }); - document(w, cx, it, None, HeadingOffset::H2) + document(w, cx, it, None, HeadingOffset::H2); } fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean::Trait) { diff --git a/src/librustdoc/html/static/css/noscript.css b/src/librustdoc/html/static/css/noscript.css index 301f03a16427a..54e8b6561f34f 100644 --- a/src/librustdoc/html/static/css/noscript.css +++ b/src/librustdoc/html/static/css/noscript.css @@ -22,3 +22,9 @@ nav.sub { .source .sidebar { display: none; } + +.notable-traits { + /* layout requires javascript + https://github.com/rust-lang/rust/issues/102576 */ + display: none; +} diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index a38c0e42ab455..44e4cc0c7acd5 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -183,6 +183,8 @@ h4.code-header { font-weight: 600; margin: 0; padding: 0; + /* position notable traits in mobile mode within the header */ + position: relative; } #crate-search, @@ -1268,13 +1270,12 @@ h3.variant { cursor: pointer; } -.notable-traits:hover .notable-traits-tooltiptext, -.notable-traits .notable-traits-tooltiptext.force-tooltip { +.notable-traits .notable-traits-tooltiptext { display: inline-block; + visibility: hidden; } -.notable-traits .notable-traits-tooltiptext { - display: none; +.notable-traits-tooltiptext { padding: 5px 3px 3px 3px; border-radius: 6px; margin-left: 5px; @@ -1292,22 +1293,26 @@ h3.variant { content: "\00a0\00a0\00a0"; } -.notable-traits .docblock { +.notable-traits-tooltiptext .docblock { margin: 0; } -.notable-traits .notable { - margin: 0; - margin-bottom: 13px; +.notable-traits-tooltiptext .notable { font-size: 1.1875rem; font-weight: 600; display: block; } -.notable-traits .docblock code.content { +.notable-traits-tooltiptext pre, .notable-traits-tooltiptext code { + background: transparent; +} + +.notable-traits-tooltiptext .docblock pre.content { margin: 0; padding: 0; font-size: 1.25rem; + white-space: pre-wrap; + overflow: hidden; } .search-failed { diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 1c84393cb4e6f..8c9d8bc34631a 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -790,6 +790,19 @@ function loadCss(cssUrl) { // we need to switch away from mobile mode and make the main content area scrollable. hideSidebar(); } + if (window.CURRENT_NOTABLE_ELEMENT) { + // As a workaround to the behavior of `contains: layout` used in doc togglers, the + // notable traits popup is positioned using javascript. + // + // This means when the window is resized, we need to redo the layout. + const base = window.CURRENT_NOTABLE_ELEMENT.NOTABLE_BASE; + const force_visible = base.NOTABLE_FORCE_VISIBLE; + hideNotable(); + if (force_visible) { + showNotable(base); + base.NOTABLE_FORCE_VISIBLE = true; + } + } }); function handleClick(id, f) { @@ -822,10 +835,78 @@ function loadCss(cssUrl) { }); }); + function showNotable(e) { + if (!window.NOTABLE_TRAITS) { + const data = document.getElementById("notable-traits-data"); + if (data) { + window.NOTABLE_TRAITS = JSON.parse(data.innerText); + } else { + throw new Error("showNotable() called on page without any notable traits!"); + } + } + if (window.CURRENT_NOTABLE_ELEMENT && window.CURRENT_NOTABLE_ELEMENT.NOTABLE_BASE === e) { + // Make this function idempotent. + return; + } + hideNotable(); + const ty = e.getAttribute("data-ty"); + const tooltip = e.getElementsByClassName("notable-traits-tooltip")[0]; + const wrapper = document.createElement("div"); + wrapper.innerHTML = "
" + window.NOTABLE_TRAITS[ty] + "
"; + wrapper.className = "notable-traits-tooltiptext"; + tooltip.appendChild(wrapper); + const pos = wrapper.getBoundingClientRect(); + tooltip.removeChild(wrapper); + wrapper.style.top = (pos.top + window.scrollY) + "px"; + wrapper.style.left = (pos.left + window.scrollX) + "px"; + wrapper.style.width = pos.width + "px"; + document.documentElement.appendChild(wrapper); + window.CURRENT_NOTABLE_ELEMENT = wrapper; + window.CURRENT_NOTABLE_ELEMENT.NOTABLE_BASE = e; + wrapper.onpointerleave = function(ev) { + // If this is a synthetic touch event, ignore it. A click event will be along shortly. + if (ev.pointerType !== "mouse") { + return; + } + if (!e.NOTABLE_FORCE_VISIBLE && !elemIsInParent(event.relatedTarget, e)) { + hideNotable(); + } + }; + } + + function hideNotable() { + if (window.CURRENT_NOTABLE_ELEMENT) { + window.CURRENT_NOTABLE_ELEMENT.NOTABLE_BASE.NOTABLE_FORCE_VISIBLE = false; + document.documentElement.removeChild(window.CURRENT_NOTABLE_ELEMENT); + window.CURRENT_NOTABLE_ELEMENT = null; + } + } + onEachLazy(document.getElementsByClassName("notable-traits"), e => { e.onclick = function() { - this.getElementsByClassName("notable-traits-tooltiptext")[0] - .classList.toggle("force-tooltip"); + this.NOTABLE_FORCE_VISIBLE = this.NOTABLE_FORCE_VISIBLE ? false : true; + if (window.CURRENT_NOTABLE_ELEMENT && !this.NOTABLE_FORCE_VISIBLE) { + hideNotable(); + } else { + showNotable(this); + } + }; + e.onpointerenter = function(ev) { + // If this is a synthetic touch event, ignore it. A click event will be along shortly. + if (ev.pointerType !== "mouse") { + return; + } + showNotable(this); + }; + e.onpointerleave = function(ev) { + // If this is a synthetic touch event, ignore it. A click event will be along shortly. + if (ev.pointerType !== "mouse") { + return; + } + if (!this.NOTABLE_FORCE_VISIBLE && + !elemIsInParent(event.relatedTarget, window.CURRENT_NOTABLE_ELEMENT)) { + hideNotable(); + } }; }); diff --git a/src/test/rustdoc-gui/notable-trait.goml b/src/test/rustdoc-gui/notable-trait.goml index efe0cb15f08a0..81d381ed1262d 100644 --- a/src/test/rustdoc-gui/notable-trait.goml +++ b/src/test/rustdoc-gui/notable-trait.goml @@ -25,22 +25,28 @@ assert-position: ( {"x": 951}, ) // The tooltip should be beside the `i` +// Also, clicking the tooltip should bring its text into the DOM +assert-count: ("//*[@class='notable-traits-tooltiptext']", 0) click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']" +assert-count: ("//*[@class='notable-traits-tooltiptext']", 1) compare-elements-position-near: ( "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']", - "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits-tooltiptext force-tooltip']", + "//*[@class='notable-traits-tooltiptext']", {"y": 2} ) compare-elements-position-false: ( "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']", - "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits-tooltiptext force-tooltip']", + "//*[@class='notable-traits-tooltiptext']", ("x") ) // The docblock should be flush with the border. assert-css: ( - "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits-tooltiptext force-tooltip']/*[@class='docblock']", + "//*[@class='notable-traits-tooltiptext']/*[@class='docblock']", {"margin-left": "0px"} ) +click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']" +move-cursor-to: "//h1" +assert-count: ("//*[@class='notable-traits-tooltiptext']", 0) // Now only the `i` should be on the next line. size: (1055, 600) @@ -98,26 +104,31 @@ assert-position: ( {"x": 289}, ) // The tooltip should be below `i` +click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']" +assert-count: ("//*[@class='notable-traits-tooltiptext']", 1) compare-elements-position-near-false: ( "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']", - "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits-tooltiptext force-tooltip']", + "//*[@class='notable-traits-tooltiptext']", {"y": 2} ) compare-elements-position-false: ( "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']", - "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits-tooltiptext force-tooltip']", + "//*[@class='notable-traits-tooltiptext']", ("x") ) compare-elements-position-near: ( - "//*[@id='method.create_an_iterator_from_read']/parent::*", - "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits-tooltiptext force-tooltip']", - {"x": 5} + "//*[@id='method.create_an_iterator_from_read']", + "//*[@class='notable-traits-tooltiptext']", + {"x": 10} ) // The docblock should be flush with the border. assert-css: ( - "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits-tooltiptext force-tooltip']/*[@class='docblock']", + "//*[@class='notable-traits-tooltiptext']/*[@class='docblock']", {"margin-left": "0px"} ) +click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']" +move-cursor-to: "//h1" +assert-count: ("//*[@class='notable-traits-tooltiptext']", 0) // Checking on very small mobile. The `i` should be on its own line. size: (365, 600) diff --git a/src/test/rustdoc/doc-notable_trait-slice.bare_fn_matches.html b/src/test/rustdoc/doc-notable_trait-slice.bare_fn_matches.html new file mode 100644 index 0000000000000..6b58be7e6853e --- /dev/null +++ b/src/test/rustdoc/doc-notable_trait-slice.bare_fn_matches.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/src/test/rustdoc/doc-notable_trait-slice.rs b/src/test/rustdoc/doc-notable_trait-slice.rs index b0d414027216a..2411da8cd4549 100644 --- a/src/test/rustdoc/doc-notable_trait-slice.rs +++ b/src/test/rustdoc/doc-notable_trait-slice.rs @@ -8,13 +8,13 @@ pub struct OtherStruct; impl SomeTrait for &[SomeStruct] {} // @has doc_notable_trait_slice/fn.bare_fn_matches.html -// @has - '//code[@class="content"]' 'impl SomeTrait for &[SomeStruct]' +// @snapshot bare_fn_matches - '//script[@id="notable-traits-data"]' pub fn bare_fn_matches() -> &'static [SomeStruct] { &[] } // @has doc_notable_trait_slice/fn.bare_fn_no_matches.html -// @!has - '//code[@class="content"]' 'impl SomeTrait for &[SomeStruct]' +// @count - '//script[@id="notable-traits-data"]' 0 pub fn bare_fn_no_matches() -> &'static [OtherStruct] { &[] } diff --git a/src/test/rustdoc/doc-notable_trait.bare-fn.html b/src/test/rustdoc/doc-notable_trait.bare-fn.html new file mode 100644 index 0000000000000..4e4a3f18f2498 --- /dev/null +++ b/src/test/rustdoc/doc-notable_trait.bare-fn.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/src/test/rustdoc/doc-notable_trait.rs b/src/test/rustdoc/doc-notable_trait.rs index 58a24b855d6e2..1f2cd7181c3d4 100644 --- a/src/test/rustdoc/doc-notable_trait.rs +++ b/src/test/rustdoc/doc-notable_trait.rs @@ -9,7 +9,8 @@ impl SomeTrait for Wrapper {} #[doc(notable_trait)] pub trait SomeTrait { // @has doc_notable_trait/trait.SomeTrait.html - // @has - '//code[@class="content"]' 'impl SomeTrait for Wrapper' + // @has - '//span[@class="notable-traits"]/@data-ty' 'Wrapper' + // @snapshot wrap-me - '//script[@id="notable-traits-data"]' fn wrap_me(self) -> Wrapper where Self: Sized { Wrapper { inner: self, @@ -22,15 +23,16 @@ impl SomeTrait for SomeStruct {} impl SomeStruct { // @has doc_notable_trait/struct.SomeStruct.html - // @has - '//code[@class="content"]' 'impl SomeTrait for SomeStruct' - // @has - '//code[@class="content"]' 'impl SomeTrait for Wrapper' + // @has - '//span[@class="notable-traits"]/@data-ty' 'SomeStruct' + // @snapshot some-struct-new - '//script[@id="notable-traits-data"]' pub fn new() -> SomeStruct { SomeStruct } } // @has doc_notable_trait/fn.bare_fn.html -// @has - '//code[@class="content"]' 'impl SomeTrait for SomeStruct' +// @has - '//span[@class="notable-traits"]/@data-ty' 'SomeStruct' +// @snapshot bare-fn - '//script[@id="notable-traits-data"]' pub fn bare_fn() -> SomeStruct { SomeStruct } diff --git a/src/test/rustdoc/doc-notable_trait.some-struct-new.html b/src/test/rustdoc/doc-notable_trait.some-struct-new.html new file mode 100644 index 0000000000000..a61e7c752e66d --- /dev/null +++ b/src/test/rustdoc/doc-notable_trait.some-struct-new.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/src/test/rustdoc/doc-notable_trait.wrap-me.html b/src/test/rustdoc/doc-notable_trait.wrap-me.html new file mode 100644 index 0000000000000..9a59d5edd12a8 --- /dev/null +++ b/src/test/rustdoc/doc-notable_trait.wrap-me.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/src/test/rustdoc/spotlight-from-dependency.odd.html b/src/test/rustdoc/spotlight-from-dependency.odd.html new file mode 100644 index 0000000000000..987a949af44b1 --- /dev/null +++ b/src/test/rustdoc/spotlight-from-dependency.odd.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/src/test/rustdoc/spotlight-from-dependency.rs b/src/test/rustdoc/spotlight-from-dependency.rs index 5245789212d8c..156aedca62b4e 100644 --- a/src/test/rustdoc/spotlight-from-dependency.rs +++ b/src/test/rustdoc/spotlight-from-dependency.rs @@ -3,7 +3,8 @@ use std::iter::Iterator; // @has foo/struct.Odd.html -// @has - '//*[@id="method.new"]//span[@class="notable-traits"]//code/span' 'impl Iterator for Odd' +// @has - '//*[@id="method.new"]//span[@class="notable-traits"]/@data-ty' 'Odd' +// @snapshot odd - '//script[@id="notable-traits-data"]' pub struct Odd { current: usize, } From a45151e2cbe74de63f50656b24257663d145c52a Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 7 Nov 2022 21:18:01 -0700 Subject: [PATCH 22/38] rustdoc: fix font color inheritance from body, and test --- src/librustdoc/html/static/js/main.js | 6 +- src/test/rustdoc-gui/notable-trait.goml | 73 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 8c9d8bc34631a..0426774e80d46 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -860,7 +860,8 @@ function loadCss(cssUrl) { wrapper.style.top = (pos.top + window.scrollY) + "px"; wrapper.style.left = (pos.left + window.scrollX) + "px"; wrapper.style.width = pos.width + "px"; - document.documentElement.appendChild(wrapper); + const body = document.getElementsByTagName("body")[0]; + body.appendChild(wrapper); window.CURRENT_NOTABLE_ELEMENT = wrapper; window.CURRENT_NOTABLE_ELEMENT.NOTABLE_BASE = e; wrapper.onpointerleave = function(ev) { @@ -877,7 +878,8 @@ function loadCss(cssUrl) { function hideNotable() { if (window.CURRENT_NOTABLE_ELEMENT) { window.CURRENT_NOTABLE_ELEMENT.NOTABLE_BASE.NOTABLE_FORCE_VISIBLE = false; - document.documentElement.removeChild(window.CURRENT_NOTABLE_ELEMENT); + const body = document.getElementsByTagName("body")[0]; + body.removeChild(window.CURRENT_NOTABLE_ELEMENT); window.CURRENT_NOTABLE_ELEMENT = null; } } diff --git a/src/test/rustdoc-gui/notable-trait.goml b/src/test/rustdoc-gui/notable-trait.goml index 81d381ed1262d..d8261d8dc902c 100644 --- a/src/test/rustdoc-gui/notable-trait.goml +++ b/src/test/rustdoc-gui/notable-trait.goml @@ -137,3 +137,76 @@ compare-elements-position-false: ( "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']", ("y", "x"), ) + +// Now check the colors. +define-function: ( + "check-colors", + (theme, header_color, content_color, type_color, trait_color), + [ + ("goto", "file://" + |DOC_PATH| + "/test_docs/struct.NotableStructWithLongName.html"), + // This is needed to ensure that the text color is computed. + ("show-text", true), + + // Setting the theme. + ("local-storage", {"rustdoc-theme": |theme|, "rustdoc-use-system-theme": "false"}), + // We reload the page so the local storage settings are being used. + ("reload"), + + ("move-cursor-to", "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"), + ("assert-count", (".notable-traits-tooltiptext", 1)), + + ("assert-css", ( + ".notable-traits-tooltiptext h3.notable", + {"color": |header_color|}, + ALL, + )), + ("assert-css", ( + ".notable-traits-tooltiptext pre.content", + {"color": |content_color|}, + ALL, + )), + ("assert-css", ( + ".notable-traits-tooltiptext pre.content a.struct", + {"color": |type_color|}, + ALL, + )), + ("assert-css", ( + ".notable-traits-tooltiptext pre.content a.trait", + {"color": |trait_color|}, + ALL, + )), + ] +) + +call-function: ( + "check-colors", + { + "theme": "ayu", + "content_color": "rgb(230, 225, 207)", + "header_color": "rgb(255, 255, 255)", + "type_color": "rgb(255, 160, 165)", + "trait_color": "rgb(57, 175, 215)", + }, +) + +call-function: ( + "check-colors", + { + "theme": "dark", + "content_color": "rgb(221, 221, 221)", + "header_color": "rgb(221, 221, 221)", + "type_color": "rgb(45, 191, 184)", + "trait_color": "rgb(183, 140, 242)", + }, +) + +call-function: ( + "check-colors", + { + "theme": "light", + "content_color": "rgb(0, 0, 0)", + "header_color": "rgb(0, 0, 0)", + "type_color": "rgb(173, 55, 138)", + "trait_color": "rgb(110, 79, 201)", + }, +) From e636af7dfdcb7d3412dde22c4c9fd71a0d9a9eed Mon Sep 17 00:00:00 2001 From: AndyJado <101876416+AndyJado@users.noreply.github.com> Date: Wed, 14 Sep 2022 23:07:19 +0800 Subject: [PATCH 23/38] lint auto pass Revert "lint auto pass" This reverts commit e58e4466384924c491a932d3f18ef50ffa5a5065. --- compiler/rustc_borrowck/src/borrow_set.rs | 2 ++ compiler/rustc_borrowck/src/constraint_generation.rs | 2 ++ compiler/rustc_borrowck/src/constraints/mod.rs | 3 +++ compiler/rustc_borrowck/src/consumers.rs | 2 ++ compiler/rustc_borrowck/src/dataflow.rs | 2 ++ compiler/rustc_borrowck/src/def_use.rs | 2 ++ compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs | 3 +++ compiler/rustc_borrowck/src/diagnostics/find_all_local_uses.rs | 3 +++ compiler/rustc_borrowck/src/diagnostics/find_use.rs | 3 +++ compiler/rustc_borrowck/src/diagnostics/var_name.rs | 3 +++ compiler/rustc_borrowck/src/facts.rs | 2 ++ compiler/rustc_borrowck/src/invalidation.rs | 2 ++ compiler/rustc_borrowck/src/location.rs | 2 ++ compiler/rustc_borrowck/src/member_constraints.rs | 2 ++ compiler/rustc_borrowck/src/nll.rs | 2 ++ compiler/rustc_borrowck/src/path_utils.rs | 2 ++ compiler/rustc_borrowck/src/place_ext.rs | 2 ++ compiler/rustc_borrowck/src/places_conflict.rs | 2 ++ compiler/rustc_borrowck/src/prefixes.rs | 2 ++ compiler/rustc_borrowck/src/region_infer/dump_mir.rs | 2 ++ compiler/rustc_borrowck/src/region_infer/graphviz.rs | 2 ++ compiler/rustc_borrowck/src/region_infer/reverse_sccs.rs | 2 ++ compiler/rustc_borrowck/src/region_infer/values.rs | 2 ++ compiler/rustc_borrowck/src/renumber.rs | 2 ++ compiler/rustc_borrowck/src/type_check/mod.rs | 2 ++ compiler/rustc_borrowck/src/used_muts.rs | 2 ++ 26 files changed, 57 insertions(+) diff --git a/compiler/rustc_borrowck/src/borrow_set.rs b/compiler/rustc_borrowck/src/borrow_set.rs index 41279588e6334..563ff056ae467 100644 --- a/compiler/rustc_borrowck/src/borrow_set.rs +++ b/compiler/rustc_borrowck/src/borrow_set.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use crate::nll::ToRegionVid; use crate::path_utils::allow_two_phase_borrow; use crate::place_ext::PlaceExt; diff --git a/compiler/rustc_borrowck/src/constraint_generation.rs b/compiler/rustc_borrowck/src/constraint_generation.rs index f185e402fc6de..11b31c3f14028 100644 --- a/compiler/rustc_borrowck/src/constraint_generation.rs +++ b/compiler/rustc_borrowck/src/constraint_generation.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_infer::infer::InferCtxt; use rustc_middle::mir::visit::TyContext; use rustc_middle::mir::visit::Visitor; diff --git a/compiler/rustc_borrowck/src/constraints/mod.rs b/compiler/rustc_borrowck/src/constraints/mod.rs index 9d9c4abb0aa57..84a93e5f72e9d 100644 --- a/compiler/rustc_borrowck/src/constraints/mod.rs +++ b/compiler/rustc_borrowck/src/constraints/mod.rs @@ -1,3 +1,6 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] + use rustc_data_structures::graph::scc::Sccs; use rustc_index::vec::IndexVec; use rustc_middle::mir::ConstraintCategory; diff --git a/compiler/rustc_borrowck/src/consumers.rs b/compiler/rustc_borrowck/src/consumers.rs index b162095f8a6cd..86da767f32273 100644 --- a/compiler/rustc_borrowck/src/consumers.rs +++ b/compiler/rustc_borrowck/src/consumers.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] //! This file provides API for compiler consumers. use rustc_hir::def_id::LocalDefId; diff --git a/compiler/rustc_borrowck/src/dataflow.rs b/compiler/rustc_borrowck/src/dataflow.rs index 9f7a4d49989ab..8070c0e6710ee 100644 --- a/compiler/rustc_borrowck/src/dataflow.rs +++ b/compiler/rustc_borrowck/src/dataflow.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_data_structures::fx::FxHashMap; use rustc_index::bit_set::BitSet; use rustc_middle::mir::{self, BasicBlock, Body, Location, Place}; diff --git a/compiler/rustc_borrowck/src/def_use.rs b/compiler/rustc_borrowck/src/def_use.rs index a5c0d77429de8..8e62a0198be46 100644 --- a/compiler/rustc_borrowck/src/def_use.rs +++ b/compiler/rustc_borrowck/src/def_use.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_middle::mir::visit::{ MutatingUseContext, NonMutatingUseContext, NonUseContext, PlaceContext, }; diff --git a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs index 897a161f78563..b99bfda1a51fe 100644 --- a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs @@ -1,3 +1,6 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] + use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed}; use rustc_infer::infer::canonical::Canonical; use rustc_infer::infer::error_reporting::nice_region_error::NiceRegionError; diff --git a/compiler/rustc_borrowck/src/diagnostics/find_all_local_uses.rs b/compiler/rustc_borrowck/src/diagnostics/find_all_local_uses.rs index b3edc35dc3642..498e9834354b7 100644 --- a/compiler/rustc_borrowck/src/diagnostics/find_all_local_uses.rs +++ b/compiler/rustc_borrowck/src/diagnostics/find_all_local_uses.rs @@ -1,3 +1,6 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] + use std::collections::BTreeSet; use rustc_middle::mir::visit::{PlaceContext, Visitor}; diff --git a/compiler/rustc_borrowck/src/diagnostics/find_use.rs b/compiler/rustc_borrowck/src/diagnostics/find_use.rs index b5a3081e56a7a..15f42e26cbf4a 100644 --- a/compiler/rustc_borrowck/src/diagnostics/find_use.rs +++ b/compiler/rustc_borrowck/src/diagnostics/find_use.rs @@ -1,3 +1,6 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] + use std::collections::VecDeque; use std::rc::Rc; diff --git a/compiler/rustc_borrowck/src/diagnostics/var_name.rs b/compiler/rustc_borrowck/src/diagnostics/var_name.rs index 9ba29f04b1a9a..b385f95b67c6f 100644 --- a/compiler/rustc_borrowck/src/diagnostics/var_name.rs +++ b/compiler/rustc_borrowck/src/diagnostics/var_name.rs @@ -1,3 +1,6 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] + use crate::Upvar; use crate::{nll::ToRegionVid, region_infer::RegionInferenceContext}; use rustc_index::vec::{Idx, IndexVec}; diff --git a/compiler/rustc_borrowck/src/facts.rs b/compiler/rustc_borrowck/src/facts.rs index 22134d5a71ce1..51ed27c167d38 100644 --- a/compiler/rustc_borrowck/src/facts.rs +++ b/compiler/rustc_borrowck/src/facts.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use crate::location::{LocationIndex, LocationTable}; use crate::BorrowIndex; use polonius_engine::AllFacts as PoloniusFacts; diff --git a/compiler/rustc_borrowck/src/invalidation.rs b/compiler/rustc_borrowck/src/invalidation.rs index 3157f861d93be..f5317a143aed7 100644 --- a/compiler/rustc_borrowck/src/invalidation.rs +++ b/compiler/rustc_borrowck/src/invalidation.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_data_structures::graph::dominators::Dominators; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{self, BasicBlock, Body, Location, NonDivergingIntrinsic, Place, Rvalue}; diff --git a/compiler/rustc_borrowck/src/location.rs b/compiler/rustc_borrowck/src/location.rs index 877944d3d70cb..9fa7e218b1b6f 100644 --- a/compiler/rustc_borrowck/src/location.rs +++ b/compiler/rustc_borrowck/src/location.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_index::vec::{Idx, IndexVec}; use rustc_middle::mir::{BasicBlock, Body, Location}; diff --git a/compiler/rustc_borrowck/src/member_constraints.rs b/compiler/rustc_borrowck/src/member_constraints.rs index 43253a2aab00c..b48f9f97daad8 100644 --- a/compiler/rustc_borrowck/src/member_constraints.rs +++ b/compiler/rustc_borrowck/src/member_constraints.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashMap; use rustc_index::vec::IndexVec; diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 4e0205f8d43a1..f8856b56d140b 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] //! The entry point of the NLL borrow checker. use rustc_data_structures::vec_map::VecMap; diff --git a/compiler/rustc_borrowck/src/path_utils.rs b/compiler/rustc_borrowck/src/path_utils.rs index b2c8dfc82c206..f8a99a2699e6f 100644 --- a/compiler/rustc_borrowck/src/path_utils.rs +++ b/compiler/rustc_borrowck/src/path_utils.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use crate::borrow_set::{BorrowData, BorrowSet, TwoPhaseActivation}; use crate::places_conflict; use crate::AccessDepth; diff --git a/compiler/rustc_borrowck/src/place_ext.rs b/compiler/rustc_borrowck/src/place_ext.rs index 93d202e49a159..9f6b1fdfcb540 100644 --- a/compiler/rustc_borrowck/src/place_ext.rs +++ b/compiler/rustc_borrowck/src/place_ext.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use crate::borrow_set::LocalsStateAtExit; use rustc_hir as hir; use rustc_middle::mir::ProjectionElem; diff --git a/compiler/rustc_borrowck/src/places_conflict.rs b/compiler/rustc_borrowck/src/places_conflict.rs index 0e71efd6f8d3e..8a87d1972ebf3 100644 --- a/compiler/rustc_borrowck/src/places_conflict.rs +++ b/compiler/rustc_borrowck/src/places_conflict.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use crate::ArtificialField; use crate::Overlap; use crate::{AccessDepth, Deep, Shallow}; diff --git a/compiler/rustc_borrowck/src/prefixes.rs b/compiler/rustc_borrowck/src/prefixes.rs index 2b50cbac9a02d..6f28134986376 100644 --- a/compiler/rustc_borrowck/src/prefixes.rs +++ b/compiler/rustc_borrowck/src/prefixes.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] //! From the NLL RFC: "The deep [aka 'supporting'] prefixes for an //! place are formed by stripping away fields and derefs, except that //! we stop when we reach the deref of a shared reference. [...] " diff --git a/compiler/rustc_borrowck/src/region_infer/dump_mir.rs b/compiler/rustc_borrowck/src/region_infer/dump_mir.rs index cc9450999525a..6524b594e44dc 100644 --- a/compiler/rustc_borrowck/src/region_infer/dump_mir.rs +++ b/compiler/rustc_borrowck/src/region_infer/dump_mir.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] //! As part of generating the regions, if you enable `-Zdump-mir=nll`, //! we will generate an annotated copy of the MIR that includes the //! state of region inference. This code handles emitting the region diff --git a/compiler/rustc_borrowck/src/region_infer/graphviz.rs b/compiler/rustc_borrowck/src/region_infer/graphviz.rs index f31ccd74ca6f7..2e15586e03b3b 100644 --- a/compiler/rustc_borrowck/src/region_infer/graphviz.rs +++ b/compiler/rustc_borrowck/src/region_infer/graphviz.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] //! This module provides linkage between RegionInferenceContext and //! `rustc_graphviz` traits, specialized to attaching borrowck analysis //! data to rendered labels. diff --git a/compiler/rustc_borrowck/src/region_infer/reverse_sccs.rs b/compiler/rustc_borrowck/src/region_infer/reverse_sccs.rs index 1e6798eee3df8..167f664609698 100644 --- a/compiler/rustc_borrowck/src/region_infer/reverse_sccs.rs +++ b/compiler/rustc_borrowck/src/region_infer/reverse_sccs.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use crate::constraints::ConstraintSccIndex; use crate::RegionInferenceContext; use itertools::Itertools; diff --git a/compiler/rustc_borrowck/src/region_infer/values.rs b/compiler/rustc_borrowck/src/region_infer/values.rs index de20a4bb465c2..7498ddccf196a 100644 --- a/compiler/rustc_borrowck/src/region_infer/values.rs +++ b/compiler/rustc_borrowck/src/region_infer/values.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_data_structures::fx::FxIndexSet; use rustc_index::bit_set::SparseBitMatrix; use rustc_index::interval::IntervalSet; diff --git a/compiler/rustc_borrowck/src/renumber.rs b/compiler/rustc_borrowck/src/renumber.rs index f3023769081f2..084754830bdbf 100644 --- a/compiler/rustc_borrowck/src/renumber.rs +++ b/compiler/rustc_borrowck/src/renumber.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_index::vec::IndexVec; use rustc_infer::infer::{InferCtxt, NllRegionVariableOrigin}; use rustc_middle::mir::visit::{MutVisitor, TyContext}; diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 9c1d0bb8b2357..6ccc29b09c0a5 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] //! This pass type-checks the MIR to ensure it is not broken. use std::rc::Rc; diff --git a/compiler/rustc_borrowck/src/used_muts.rs b/compiler/rustc_borrowck/src/used_muts.rs index 8833753b12c5d..e297b1230ea0c 100644 --- a/compiler/rustc_borrowck/src/used_muts.rs +++ b/compiler/rustc_borrowck/src/used_muts.rs @@ -1,3 +1,5 @@ +#![deny(rustc::untranslatable_diagnostic)] +#![deny(rustc::diagnostic_outside_of_impl)] use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir::visit::{PlaceContext, Visitor}; use rustc_middle::mir::{ From a0cee0ab90c7ecf39e14958e5f325864a5f496fe Mon Sep 17 00:00:00 2001 From: AndyJado <101876416+AndyJado@users.noreply.github.com> Date: Fri, 4 Nov 2022 17:09:14 +0800 Subject: [PATCH 24/38] remove old var_span_path_only doc comment --- .../src/diagnostics/conflict_errors.rs | 15 ++----- .../rustc_borrowck/src/diagnostics/mod.rs | 33 +++++++++++--- .../rustc_borrowck/src/session_diagnostics.rs | 44 +++++++++++++++++++ .../locales/en-US/borrowck.ftl | 24 ++++++++++ 4 files changed, 99 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 7f26e970e3094..46c0bbac2f156 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -224,10 +224,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - use_spans.var_span_label_path_only( - &mut err, - format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()), - ); + use_spans.var_path_only_subdiag(&mut err, desired_action); if !is_loop_move { err.span_label( @@ -404,10 +401,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let used = desired_action.as_general_verb_in_past_tense(); let mut err = struct_span_err!(self, span, E0381, "{used} binding {desc}{isnt_initialized}"); - use_spans.var_span_label_path_only( - &mut err, - format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()), - ); + use_spans.var_path_only_subdiag(&mut err, desired_action); if let InitializationRequiringAction::PartialAssignment | InitializationRequiringAction::Assignment = desired_action @@ -678,10 +672,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { err.span_label(borrow_span, format!("borrow of {} occurs here", borrow_msg)); err.span_label(span, format!("move out of {} occurs here", value_msg)); - borrow_spans.var_span_label_path_only( - &mut err, - format!("borrow occurs due to use{}", borrow_spans.describe()), - ); + borrow_spans.var_path_only_subdiag(&mut err, crate::InitializationRequiringAction::Borrow); move_spans.var_span_label( &mut err, diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 61518378e3d0c..c7b9f617d5a52 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -595,11 +595,34 @@ impl UseSpans<'_> { } } - // Add a span label to the use of the captured variable, if it exists. - // only adds label to the `path_span` - pub(super) fn var_span_label_path_only(self, err: &mut Diagnostic, message: impl Into) { - if let UseSpans::ClosureUse { path_span, .. } = self { - err.span_label(path_span, message); + /// Add a span label to the use of the captured variable, if it exists. + /// only adds label to the `path_span` + pub(super) fn var_path_only_subdiag( + self, + err: &mut Diagnostic, + action: crate::InitializationRequiringAction, + ) { + use crate::session_diagnostics::CaptureVarPathUseCause::*; + use crate::InitializationRequiringAction::*; + if let UseSpans::ClosureUse { generator_kind, path_span, .. } = self { + match generator_kind { + Some(_) => { + err.subdiagnostic(match action { + Borrow => BorrowInGenerator { path_span }, + MatchOn | Use => UseInGenerator { path_span }, + Assignment => AssignInGenerator { path_span }, + PartialAssignment => AssignPartInGenerator { path_span }, + }); + } + None => { + err.subdiagnostic(match action { + Borrow => BorrowInClosure { path_span }, + MatchOn | Use => UseInClosure { path_span }, + Assignment => AssignInClosure { path_span }, + PartialAssignment => AssignPartInClosure { path_span }, + }); + } + } } } diff --git a/compiler/rustc_borrowck/src/session_diagnostics.rs b/compiler/rustc_borrowck/src/session_diagnostics.rs index 824f20a31bb09..62c11e303b800 100644 --- a/compiler/rustc_borrowck/src/session_diagnostics.rs +++ b/compiler/rustc_borrowck/src/session_diagnostics.rs @@ -178,3 +178,47 @@ pub(crate) enum CaptureVarCause { var_span: Span, }, } + +#[derive(Subdiagnostic)] +pub(crate) enum CaptureVarPathUseCause { + #[label(borrowck_borrow_due_to_use_generator)] + BorrowInGenerator { + #[primary_span] + path_span: Span, + }, + #[label(borrowck_use_due_to_use_generator)] + UseInGenerator { + #[primary_span] + path_span: Span, + }, + #[label(borrowck_assign_due_to_use_generator)] + AssignInGenerator { + #[primary_span] + path_span: Span, + }, + #[label(borrowck_assign_part_due_to_use_generator)] + AssignPartInGenerator { + #[primary_span] + path_span: Span, + }, + #[label(borrowck_borrow_due_to_use_closure)] + BorrowInClosure { + #[primary_span] + path_span: Span, + }, + #[label(borrowck_use_due_to_use_closure)] + UseInClosure { + #[primary_span] + path_span: Span, + }, + #[label(borrowck_assign_due_to_use_closure)] + AssignInClosure { + #[primary_span] + path_span: Span, + }, + #[label(borrowck_assign_part_due_to_use_closure)] + AssignPartInClosure { + #[primary_span] + path_span: Span, + }, +} diff --git a/compiler/rustc_error_messages/locales/en-US/borrowck.ftl b/compiler/rustc_error_messages/locales/en-US/borrowck.ftl index 80fc4c6e4f5d3..5d6617d5bcc72 100644 --- a/compiler/rustc_error_messages/locales/en-US/borrowck.ftl +++ b/compiler/rustc_error_messages/locales/en-US/borrowck.ftl @@ -70,3 +70,27 @@ borrowck_var_borrow_by_use_place_in_closure = borrowck_var_borrow_by_use_place = borrow occurs due to use of {$place} + +borrowck_borrow_due_to_use_generator = + borrow occurs due to use in generator + +borrowck_use_due_to_use_generator = + use occurs due to use in generator + +borrowck_assign_due_to_use_generator = + assign occurs due to use in generator + +borrowck_assign_part_due_to_use_generator = + assign to part occurs due to use in generator + +borrowck_borrow_due_to_use_closure = + borrow occurs due to use in closure + +borrowck_use_due_to_use_closure = + use occurs due to use in closure + +borrowck_assign_due_to_use_closure = + assign occurs due to use in closure + +borrowck_assign_part_due_to_use_closure = + assign to part occurs due to use in closure From abf259cc541214d1a08c8e8f74c2ce44fc310f76 Mon Sep 17 00:00:00 2001 From: AndyJado <101876416+AndyJado@users.noreply.github.com> Date: Wed, 9 Nov 2022 20:56:28 +0800 Subject: [PATCH 25/38] var_subdiag refinement trim old --- .../src/diagnostics/conflict_errors.rs | 25 +++---- .../rustc_borrowck/src/diagnostics/mod.rs | 31 +++++---- .../rustc_borrowck/src/session_diagnostics.rs | 65 ++++++++++--------- .../locales/en-US/borrowck.ftl | 15 +++++ compiler/rustc_middle/src/mir/mod.rs | 1 + 5 files changed, 80 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 46c0bbac2f156..60860f93daa3c 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -715,22 +715,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { borrow_span, &self.describe_any_place(borrow.borrowed_place.as_ref()), ); - borrow_spans.var_subdiag( - &mut err, - |var_span| { - use crate::session_diagnostics::CaptureVarCause::*; - let place = &borrow.borrowed_place; - let desc_place = self.describe_any_place(place.as_ref()); - match borrow_spans { - UseSpans::ClosureUse { generator_kind, .. } => match generator_kind { - Some(_) => BorrowUsePlaceGenerator { place: desc_place, var_span }, - None => BorrowUsePlaceClosure { place: desc_place, var_span }, - }, - _ => BorrowUsePlace { place: desc_place, var_span }, - } - }, - "mutable", - ); + borrow_spans.var_subdiag(&mut err, Some(borrow.kind), |kind, var_span| { + use crate::session_diagnostics::CaptureVarCause::*; + let place = &borrow.borrowed_place; + let desc_place = self.describe_any_place(place.as_ref()); + match kind { + Some(_) => BorrowUsePlaceGenerator { place: desc_place, var_span }, + None => BorrowUsePlaceClosure { place: desc_place, var_span }, + } + }); self.explain_why_borrow_contains_point(location, borrow, None) .add_explanation_to_diagnostic( diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index c7b9f617d5a52..7f26af67c71b2 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -650,19 +650,28 @@ impl UseSpans<'_> { pub(super) fn var_subdiag( self, err: &mut Diagnostic, - f: impl Fn(Span) -> crate::session_diagnostics::CaptureVarCause, - kind_desc: impl Into, + kind: Option, + f: impl Fn(Option, Span) -> crate::session_diagnostics::CaptureVarCause, ) { - if let UseSpans::ClosureUse { capture_kind_span, path_span, .. } = self { - if capture_kind_span == path_span { - err.subdiagnostic(f(capture_kind_span)); - } else { - err.subdiagnostic(crate::session_diagnostics::CaptureVarKind { - kind_desc: kind_desc.into(), - kind_span: capture_kind_span, + use crate::session_diagnostics::CaptureVarKind::*; + if let UseSpans::ClosureUse { generator_kind, capture_kind_span, path_span, .. } = self { + if capture_kind_span != path_span { + err.subdiagnostic(match kind { + Some(kd) => match kd { + rustc_middle::mir::BorrowKind::Shared + | rustc_middle::mir::BorrowKind::Shallow + | rustc_middle::mir::BorrowKind::Unique => { + Immute { kind_span: capture_kind_span } + } + + rustc_middle::mir::BorrowKind::Mut { .. } => { + Mut { kind_span: capture_kind_span } + } + }, + None => Move { kind_span: capture_kind_span }, }); - err.subdiagnostic(f(path_span)); - } + }; + err.subdiagnostic(f(generator_kind, path_span)); } } diff --git a/compiler/rustc_borrowck/src/session_diagnostics.rs b/compiler/rustc_borrowck/src/session_diagnostics.rs index 62c11e303b800..3f9bfc5373aea 100644 --- a/compiler/rustc_borrowck/src/session_diagnostics.rs +++ b/compiler/rustc_borrowck/src/session_diagnostics.rs @@ -149,36 +149,6 @@ pub(crate) enum RequireStaticErr { }, } -#[derive(Subdiagnostic)] -#[label(borrowck_capture_kind_label)] -pub(crate) struct CaptureVarKind { - pub kind_desc: String, - #[primary_span] - pub kind_span: Span, -} - -#[derive(Subdiagnostic)] -pub(crate) enum CaptureVarCause { - #[label(borrowck_var_borrow_by_use_place)] - BorrowUsePlace { - place: String, - #[primary_span] - var_span: Span, - }, - #[label(borrowck_var_borrow_by_use_place_in_generator)] - BorrowUsePlaceGenerator { - place: String, - #[primary_span] - var_span: Span, - }, - #[label(borrowck_var_borrow_by_use_place_in_closure)] - BorrowUsePlaceClosure { - place: String, - #[primary_span] - var_span: Span, - }, -} - #[derive(Subdiagnostic)] pub(crate) enum CaptureVarPathUseCause { #[label(borrowck_borrow_due_to_use_generator)] @@ -222,3 +192,38 @@ pub(crate) enum CaptureVarPathUseCause { path_span: Span, }, } + +#[derive(Subdiagnostic)] +pub(crate) enum CaptureVarKind { + #[label(borrowck_capture_immute)] + Immute { + #[primary_span] + kind_span: Span, + }, + #[label(borrowck_capture_mut)] + Mut { + #[primary_span] + kind_span: Span, + }, + #[label(borrowck_capture_move)] + Move { + #[primary_span] + kind_span: Span, + }, +} + +#[derive(Subdiagnostic)] +pub(crate) enum CaptureVarCause { + #[label(borrowck_var_borrow_by_use_place_in_generator)] + BorrowUsePlaceGenerator { + place: String, + #[primary_span] + var_span: Span, + }, + #[label(borrowck_var_borrow_by_use_place_in_closure)] + BorrowUsePlaceClosure { + place: String, + #[primary_span] + var_span: Span, + }, +} diff --git a/compiler/rustc_error_messages/locales/en-US/borrowck.ftl b/compiler/rustc_error_messages/locales/en-US/borrowck.ftl index 5d6617d5bcc72..e3174360c90e2 100644 --- a/compiler/rustc_error_messages/locales/en-US/borrowck.ftl +++ b/compiler/rustc_error_messages/locales/en-US/borrowck.ftl @@ -94,3 +94,18 @@ borrowck_assign_due_to_use_closure = borrowck_assign_part_due_to_use_closure = assign to part occurs due to use in closure + +borrowck_capture_immute = + capture is immutable because of use here + +borrowck_capture_mut = + capture is mutable because of use here + +borrowck_capture_move = + capture is moved because of use here + +borrowck_var_move_by_use_place_in_generator = + move occurs due to use of {$place} in generator + +borrowck_var_move_by_use_place_in_closure = + move occurs due to use of {$place} in closure diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 0a96d23e3543b..35675313b48bc 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1898,6 +1898,7 @@ impl BorrowKind { } } + // FIXME: won't be used after diagnostic migration pub fn describe_mutability(&self) -> &str { match *self { BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => "immutable", From 057d8e5c43eb63691c5c6fbae8e67070150190f4 Mon Sep 17 00:00:00 2001 From: AndyJado <101876416+AndyJado@users.noreply.github.com> Date: Wed, 9 Nov 2022 20:57:44 +0800 Subject: [PATCH 26/38] struct error E0505 --- compiler/rustc_borrowck/src/borrowck_errors.rs | 13 +++++++++++-- .../src/diagnostics/conflict_errors.rs | 11 +++++++---- compiler/rustc_borrowck/src/session_diagnostics.rs | 13 +++++++++++++ .../locales/en-US/borrowck.ftl | 14 ++++++++++++++ 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_borrowck/src/borrowck_errors.rs b/compiler/rustc_borrowck/src/borrowck_errors.rs index 08ea00d71ef9d..01be379120dc7 100644 --- a/compiler/rustc_borrowck/src/borrowck_errors.rs +++ b/compiler/rustc_borrowck/src/borrowck_errors.rs @@ -8,9 +8,18 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> { pub(crate) fn cannot_move_when_borrowed( &self, span: Span, - desc: &str, + borrow_span: Span, + place: &str, + borrow_place: &str, + value_place: &str, ) -> DiagnosticBuilder<'cx, ErrorGuaranteed> { - struct_span_err!(self, span, E0505, "cannot move out of {} because it is borrowed", desc,) + self.infcx.tcx.sess.create_err(crate::session_diagnostics::MoveBorrow { + place, + span, + borrow_place, + value_place, + borrow_span, + }) } pub(crate) fn cannot_use_when_mutably_borrowed( diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 60860f93daa3c..0f76cd1cdc124 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -667,10 +667,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let move_spans = self.move_spans(place.as_ref(), location); let span = move_spans.args_or_use(); - let mut err = - self.cannot_move_when_borrowed(span, &self.describe_any_place(place.as_ref())); - err.span_label(borrow_span, format!("borrow of {} occurs here", borrow_msg)); - err.span_label(span, format!("move out of {} occurs here", value_msg)); + let mut err = self.cannot_move_when_borrowed( + span, + borrow_span, + &self.describe_any_place(place.as_ref()), + &borrow_msg, + &value_msg, + ); borrow_spans.var_path_only_subdiag(&mut err, crate::InitializationRequiringAction::Borrow); diff --git a/compiler/rustc_borrowck/src/session_diagnostics.rs b/compiler/rustc_borrowck/src/session_diagnostics.rs index 3f9bfc5373aea..577332c0744b8 100644 --- a/compiler/rustc_borrowck/src/session_diagnostics.rs +++ b/compiler/rustc_borrowck/src/session_diagnostics.rs @@ -227,3 +227,16 @@ pub(crate) enum CaptureVarCause { var_span: Span, }, } + +#[derive(Diagnostic)] +#[diag(borrowck_cannot_move_when_borrowed, code = "E0505")] +pub(crate) struct MoveBorrow<'a> { + pub place: &'a str, + pub borrow_place: &'a str, + pub value_place: &'a str, + #[primary_span] + #[label(move_label)] + pub span: Span, + #[label] + pub borrow_span: Span, +} diff --git a/compiler/rustc_error_messages/locales/en-US/borrowck.ftl b/compiler/rustc_error_messages/locales/en-US/borrowck.ftl index e3174360c90e2..de47ada826444 100644 --- a/compiler/rustc_error_messages/locales/en-US/borrowck.ftl +++ b/compiler/rustc_error_messages/locales/en-US/borrowck.ftl @@ -109,3 +109,17 @@ borrowck_var_move_by_use_place_in_generator = borrowck_var_move_by_use_place_in_closure = move occurs due to use of {$place} in closure + +borrowck_cannot_move_when_borrowed = + cannot move out of {$place -> + [value] value + *[other] {$place} + } because it is borrowed + .label = borrow of {$borrow_place -> + [value] value + *[other] {$borrow_place} + } occurs here + .move_label = move out of {$value_place -> + [value] value + *[other] {$value_place} + } occurs here From 53e8b490d82a1abe0c617404fe1b352579cfbbc0 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Wed, 9 Nov 2022 13:55:52 -0700 Subject: [PATCH 27/38] rustdoc: sort output to make it deterministic --- src/librustdoc/html/render/mod.rs | 3 ++- src/test/rustdoc/doc-notable_trait.some-struct-new.html | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 1c13e2bf67781..fffd90c51fad1 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1387,7 +1387,8 @@ pub(crate) fn notable_traits_json<'a>( tys: impl Iterator, cx: &Context<'_>, ) -> String { - let mp: Vec<(String, String)> = tys.map(|ty| notable_traits_decl(ty, cx)).collect(); + let mut mp: Vec<(String, String)> = tys.map(|ty| notable_traits_decl(ty, cx)).collect(); + mp.sort_by(|(name1, _html1), (name2, _html2)| name1.cmp(name2)); struct NotableTraitsMap(Vec<(String, String)>); impl Serialize for NotableTraitsMap { fn serialize(&self, serializer: S) -> Result diff --git a/src/test/rustdoc/doc-notable_trait.some-struct-new.html b/src/test/rustdoc/doc-notable_trait.some-struct-new.html index a61e7c752e66d..c0fd9748fd371 100644 --- a/src/test/rustdoc/doc-notable_trait.some-struct-new.html +++ b/src/test/rustdoc/doc-notable_trait.some-struct-new.html @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file From 5c25d30f6fe9996f815a96f4b328e62c452cc3e3 Mon Sep 17 00:00:00 2001 From: Ben Reeves Date: Thu, 24 Mar 2022 19:24:40 -0500 Subject: [PATCH 28/38] Allow specialized const trait impls. Fixes #95186. Fixes #95187. --- .../src/impl_wf_check/min_specialization.rs | 65 +++++++++++++------ .../const-default-const-specialized.rs | 38 +++++++++++ .../const-default-non-const-specialized.rs | 37 +++++++++++ ...const-default-non-const-specialized.stderr | 37 +++++++++++ .../specialization/default-keyword.rs | 14 ++++ .../issue-95186-specialize-on-tilde-const.rs | 34 ++++++++++ ...87-same-trait-bound-different-constness.rs | 28 ++++++++ .../non-const-default-const-specialized.rs | 34 ++++++++++ ...non-const-default-const-specialized.stderr | 20 ++++++ 9 files changed, 286 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.rs create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/default-keyword.rs create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.rs create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr diff --git a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs index 267077cdab4e6..f65760b9c98ca 100644 --- a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs +++ b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs @@ -80,6 +80,7 @@ use rustc_span::Span; use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt; use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _; use rustc_trait_selection::traits::{self, translate_substs, wf, ObligationCtxt}; +use tracing::instrument; pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) { if let Some(node) = parent_specialization_node(tcx, impl_def_id) { @@ -103,13 +104,11 @@ fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId) -> Opti } /// Check that `impl1` is a sound specialization +#[instrument(level = "debug", skip(tcx))] fn check_always_applicable(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node) { if let Some((impl1_substs, impl2_substs)) = get_impl_substs(tcx, impl1_def_id, impl2_node) { let impl2_def_id = impl2_node.def_id(); - debug!( - "check_always_applicable(\nimpl1_def_id={:?},\nimpl2_def_id={:?},\nimpl2_substs={:?}\n)", - impl1_def_id, impl2_def_id, impl2_substs - ); + debug!(?impl2_def_id, ?impl2_substs); let parent_substs = if impl2_node.is_from_trait() { impl2_substs.to_vec() @@ -280,13 +279,13 @@ fn check_static_lifetimes<'tcx>( /// /// Each predicate `P` must be: /// -/// * global (not reference any parameters) -/// * `T: Tr` predicate where `Tr` is an always-applicable trait -/// * on the base `impl impl2` -/// * Currently this check is done using syntactic equality, which is -/// conservative but generally sufficient. -/// * a well-formed predicate of a type argument of the trait being implemented, +/// * Global (not reference any parameters). +/// * A `T: Tr` predicate where `Tr` is an always-applicable trait. +/// * Present on the base impl `impl2`. +/// * This check is done using the `trait_predicates_eq` function below. +/// * A well-formed predicate of a type argument of the trait being implemented, /// including the `Self`-type. +#[instrument(level = "debug", skip(tcx))] fn check_predicates<'tcx>( tcx: TyCtxt<'tcx>, impl1_def_id: LocalDefId, @@ -322,10 +321,7 @@ fn check_predicates<'tcx>( .map(|obligation| obligation.predicate) .collect() }; - debug!( - "check_always_applicable(\nimpl1_predicates={:?},\nimpl2_predicates={:?}\n)", - impl1_predicates, impl2_predicates, - ); + debug!(?impl1_predicates, ?impl2_predicates); // Since impls of always applicable traits don't get to assume anything, we // can also assume their supertraits apply. @@ -373,25 +369,52 @@ fn check_predicates<'tcx>( ); for (predicate, span) in impl1_predicates { - if !impl2_predicates.contains(&predicate) { + if !impl2_predicates.iter().any(|pred2| trait_predicates_eq(predicate, *pred2)) { check_specialization_on(tcx, predicate, span) } } } +/// Checks whether two predicates are the same for the purposes of specialization. +/// +/// This is slightly more complicated than simple syntactic equivalence, since +/// we want to equate `T: Tr` with `T: ~const Tr` so this can work: +/// +/// #[rustc_specialization_trait] +/// trait Specialize { } +/// +/// impl const Tr for T { } +/// impl Tr for T { } +fn trait_predicates_eq<'tcx>( + predicate1: ty::Predicate<'tcx>, + predicate2: ty::Predicate<'tcx>, +) -> bool { + let predicate_kind_without_constness = |kind: ty::PredicateKind<'tcx>| match kind { + ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _, polarity }) => { + ty::PredicateKind::Trait(ty::TraitPredicate { + trait_ref, + constness: ty::BoundConstness::NotConst, + polarity, + }) + } + _ => kind, + }; + + let pred1_kind_not_const = predicate1.kind().map_bound(predicate_kind_without_constness); + let pred2_kind_not_const = predicate2.kind().map_bound(predicate_kind_without_constness); + + pred1_kind_not_const == pred2_kind_not_const +} + +#[instrument(level = "debug", skip(tcx))] fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, predicate: ty::Predicate<'tcx>, span: Span) { - debug!("can_specialize_on(predicate = {:?})", predicate); match predicate.kind().skip_binder() { // Global predicates are either always true or always false, so we // are fine to specialize on. _ if predicate.is_global() => (), // We allow specializing on explicitly marked traits with no associated // items. - ty::PredicateKind::Trait(ty::TraitPredicate { - trait_ref, - constness: ty::BoundConstness::NotConst, - polarity: _, - }) => { + ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _, polarity: _ }) => { if !matches!( trait_predicate_kind(tcx, predicate), Some(TraitSpecializationKind::Marker) diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.rs new file mode 100644 index 0000000000000..1eddfbf50f386 --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.rs @@ -0,0 +1,38 @@ +// Tests that a const default trait impl can be specialized by another const +// trait impl and that the specializing impl will be used during const-eval. + +// run-pass + +#![feature(const_trait_impl)] +#![feature(min_specialization)] + +trait Value { + fn value() -> u32; +} + +const fn get_value() -> u32 { + T::value() +} + +impl const Value for T { + default fn value() -> u32 { + 0 + } +} + +struct FortyTwo; + +impl const Value for FortyTwo { + fn value() -> u32 { + 42 + } +} + +const ZERO: u32 = get_value::<()>(); + +const FORTY_TWO: u32 = get_value::(); + +fn main() { + assert_eq!(ZERO, 0); + assert_eq!(FORTY_TWO, 42); +} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs new file mode 100644 index 0000000000000..31de6fadeb7a2 --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs @@ -0,0 +1,37 @@ +// Tests that a const default trait impl can be specialized by a non-const trait +// impl, but that the specializing impl cannot be used in a const context. + +#![feature(const_trait_impl)] +#![feature(min_specialization)] + +trait Value { + fn value() -> u32; +} + +const fn get_value() -> u32 { + T::value() + //~^ ERROR any use of this value will cause an error [const_err] + //~| WARNING this was previously accepted +} + +impl const Value for T { + default fn value() -> u32 { + 0 + } +} + +struct FortyTwo; + +impl Value for FortyTwo { + fn value() -> u32 { + println!("You can't do that (constly)"); + 42 + } +} + +const ZERO: u32 = get_value::<()>(); + +const FORTY_TWO: u32 = + get_value::(); // This is the line that causes the error, but it gets reported above + +fn main() {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr new file mode 100644 index 0000000000000..7dfd489ea65c6 --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr @@ -0,0 +1,37 @@ +error: any use of this value will cause an error + --> $DIR/const-default-non-const-specialized.rs:12:5 + | +LL | T::value() + | ^^^^^^^^^^ + | | + | calling non-const function `::value` + | inside `get_value::` at $DIR/const-default-non-const-specialized.rs:12:5 + | inside `FORTY_TWO` at $DIR/const-default-non-const-specialized.rs:35:5 +... +LL | const FORTY_TWO: u32 = + | -------------------- + | + = note: `#[deny(const_err)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #71800 + +error: aborting due to previous error + +Future incompatibility report: Future breakage diagnostic: +error: any use of this value will cause an error + --> $DIR/const-default-non-const-specialized.rs:12:5 + | +LL | T::value() + | ^^^^^^^^^^ + | | + | calling non-const function `::value` + | inside `get_value::` at $DIR/const-default-non-const-specialized.rs:12:5 + | inside `FORTY_TWO` at $DIR/const-default-non-const-specialized.rs:35:5 +... +LL | const FORTY_TWO: u32 = + | -------------------- + | + = note: `#[deny(const_err)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #71800 + diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/default-keyword.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/default-keyword.rs new file mode 100644 index 0000000000000..c03b0a0d19ca5 --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/default-keyword.rs @@ -0,0 +1,14 @@ +// check-pass + +#![feature(const_trait_impl)] +#![feature(min_specialization)] + +trait Foo { + fn foo(); +} + +impl const Foo for u32 { + default fn foo() {} +} + +fn main() {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.rs new file mode 100644 index 0000000000000..1f7f47879d78b --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.rs @@ -0,0 +1,34 @@ +// Tests that `~const` trait bounds can be used to specialize const trait impls. + +// check-pass + +#![feature(const_trait_impl)] +#![feature(rustc_attrs)] +#![feature(min_specialization)] + +#[rustc_specialization_trait] +trait Specialize {} + +trait Foo {} + +impl const Foo for T {} + +impl const Foo for T +where + T: ~const Specialize, +{} + +trait Bar {} + +impl const Bar for T +where + T: ~const Foo, +{} + +impl const Bar for T +where + T: ~const Foo, + T: ~const Specialize, +{} + +fn main() {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs new file mode 100644 index 0000000000000..f6daba5595a8d --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs @@ -0,0 +1,28 @@ +// Tests that `T: ~const Foo` and `T: Foo` are treated as equivalent for the +// purposes of min_specialization. + +// check-pass + +#![feature(rustc_attrs)] +#![feature(min_specialization)] +#![feature(const_trait_impl)] + +#[rustc_specialization_trait] +trait Specialize {} + +trait Foo {} + +trait Bar {} + +impl const Bar for T +where + T: ~const Foo, +{} + +impl Bar for T +where + T: Foo, + T: Specialize, +{} + +fn main() {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs new file mode 100644 index 0000000000000..cf6c292e8a465 --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs @@ -0,0 +1,34 @@ +// Tests that a non-const default impl can be specialized by a const trait impl, +// but that the default impl cannot be used in a const context. + +#![feature(const_trait_impl)] +#![feature(min_specialization)] + +trait Value { + fn value() -> u32; +} + +const fn get_value() -> u32 { + T::value() +} + +impl Value for T { + default fn value() -> u32 { + println!("You can't do that (constly)"); + 0 + } +} + +struct FortyTwo; + +impl const Value for FortyTwo { + fn value() -> u32 { + 42 + } +} + +const ZERO: u32 = get_value::<()>(); //~ ERROR the trait bound `(): ~const Value` is not satisfied + +const FORTY_TWO: u32 = get_value::(); + +fn main() {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr new file mode 100644 index 0000000000000..1065009c8910e --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr @@ -0,0 +1,20 @@ +error[E0277]: the trait bound `(): ~const Value` is not satisfied + --> $DIR/non-const-default-const-specialized.rs:30:31 + | +LL | const ZERO: u32 = get_value::<()>(); + | ^^ the trait `~const Value` is not implemented for `()` + | +note: the trait `Value` is implemented for `()`, but that implementation is not `const` + --> $DIR/non-const-default-const-specialized.rs:30:31 + | +LL | const ZERO: u32 = get_value::<()>(); + | ^^ +note: required by a bound in `get_value` + --> $DIR/non-const-default-const-specialized.rs:11:23 + | +LL | const fn get_value() -> u32 { + | ^^^^^^^^^^^^ required by this bound in `get_value` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. From ce03d259da4a9e47111465353363597868aa2266 Mon Sep 17 00:00:00 2001 From: Ben Reeves Date: Tue, 13 Sep 2022 22:18:14 -0500 Subject: [PATCH 29/38] Disallow specializing on const impls with non-const impls. --- .../src/impl_wf_check/min_specialization.rs | 35 ++++++++++++++++-- .../const-default-non-const-specialized.rs | 17 ++------- ...const-default-non-const-specialized.stderr | 37 ++----------------- ...87-same-trait-bound-different-constness.rs | 10 ++--- .../non-const-default-const-specialized.rs | 12 ++++-- ...non-const-default-const-specialized.stderr | 20 ---------- 6 files changed, 51 insertions(+), 80 deletions(-) delete mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr diff --git a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs index f65760b9c98ca..9c7150b79240a 100644 --- a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs +++ b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs @@ -69,6 +69,7 @@ use crate::constrained_generic_params as cgp; use crate::errors::SubstsOnOverriddenImpl; use rustc_data_structures::fx::FxHashSet; +use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_infer::infer::outlives::env::OutlivesEnvironment; use rustc_infer::infer::TyCtxtInferExt; @@ -117,12 +118,33 @@ fn check_always_applicable(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node }; let span = tcx.def_span(impl1_def_id); + check_constness(tcx, impl1_def_id, impl2_node, span); check_static_lifetimes(tcx, &parent_substs, span); check_duplicate_params(tcx, impl1_substs, &parent_substs, span); check_predicates(tcx, impl1_def_id, impl1_substs, impl2_node, impl2_substs, span); } } +/// Check that the specializing impl `impl1` is at least as const as the base +/// impl `impl2` +fn check_constness(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node, span: Span) { + if impl2_node.is_from_trait() { + // This isn't a specialization + return; + } + + let impl1_constness = tcx.constness(impl1_def_id.to_def_id()); + let impl2_constness = tcx.constness(impl2_node.def_id()); + + if let hir::Constness::Const = impl2_constness { + if let hir::Constness::NotConst = impl1_constness { + tcx.sess + .struct_span_err(span, "cannot specialize on const impl with non-const impl") + .emit(); + } + } +} + /// Given a specializing impl `impl1`, and the base impl `impl2`, returns two /// substitutions `(S1, S2)` that equate their trait references. The returned /// types are expressed in terms of the generics of `impl1`. @@ -277,7 +299,7 @@ fn check_static_lifetimes<'tcx>( /// Check whether predicates on the specializing impl (`impl1`) are allowed. /// -/// Each predicate `P` must be: +/// Each predicate `P` must be one of: /// /// * Global (not reference any parameters). /// * A `T: Tr` predicate where `Tr` is an always-applicable trait. @@ -375,16 +397,19 @@ fn check_predicates<'tcx>( } } -/// Checks whether two predicates are the same for the purposes of specialization. +/// Checks if some predicate on the specializing impl (`predicate1`) is the same +/// as some predicate on the base impl (`predicate2`). /// /// This is slightly more complicated than simple syntactic equivalence, since /// we want to equate `T: Tr` with `T: ~const Tr` so this can work: /// +/// ```ignore (illustrative) /// #[rustc_specialization_trait] /// trait Specialize { } /// -/// impl const Tr for T { } -/// impl Tr for T { } +/// impl Tr for T { } +/// impl const Tr for T { } +/// ``` fn trait_predicates_eq<'tcx>( predicate1: ty::Predicate<'tcx>, predicate2: ty::Predicate<'tcx>, @@ -400,6 +425,8 @@ fn trait_predicates_eq<'tcx>( _ => kind, }; + // We rely on `check_constness` above to ensure that pred1 is const if pred2 + // is const. let pred1_kind_not_const = predicate1.kind().map_bound(predicate_kind_without_constness); let pred2_kind_not_const = predicate2.kind().map_bound(predicate_kind_without_constness); diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs index 31de6fadeb7a2..a25329ba388d2 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs @@ -1,5 +1,5 @@ -// Tests that a const default trait impl can be specialized by a non-const trait -// impl, but that the specializing impl cannot be used in a const context. +// Tests that a const default trait impl cannot be specialized by a non-const +// trait impl. #![feature(const_trait_impl)] #![feature(min_specialization)] @@ -8,12 +8,6 @@ trait Value { fn value() -> u32; } -const fn get_value() -> u32 { - T::value() - //~^ ERROR any use of this value will cause an error [const_err] - //~| WARNING this was previously accepted -} - impl const Value for T { default fn value() -> u32 { 0 @@ -22,16 +16,11 @@ impl const Value for T { struct FortyTwo; -impl Value for FortyTwo { +impl Value for FortyTwo { //~ ERROR cannot specialize on const impl with non-const impl fn value() -> u32 { println!("You can't do that (constly)"); 42 } } -const ZERO: u32 = get_value::<()>(); - -const FORTY_TWO: u32 = - get_value::(); // This is the line that causes the error, but it gets reported above - fn main() {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr index 7dfd489ea65c6..b0b76e7eca8ac 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr @@ -1,37 +1,8 @@ -error: any use of this value will cause an error - --> $DIR/const-default-non-const-specialized.rs:12:5 +error: cannot specialize on const impl with non-const impl + --> $DIR/const-default-non-const-specialized.rs:19:1 | -LL | T::value() - | ^^^^^^^^^^ - | | - | calling non-const function `::value` - | inside `get_value::` at $DIR/const-default-non-const-specialized.rs:12:5 - | inside `FORTY_TWO` at $DIR/const-default-non-const-specialized.rs:35:5 -... -LL | const FORTY_TWO: u32 = - | -------------------- - | - = note: `#[deny(const_err)]` on by default - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #71800 +LL | impl Value for FortyTwo { + | ^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error -Future incompatibility report: Future breakage diagnostic: -error: any use of this value will cause an error - --> $DIR/const-default-non-const-specialized.rs:12:5 - | -LL | T::value() - | ^^^^^^^^^^ - | | - | calling non-const function `::value` - | inside `get_value::` at $DIR/const-default-non-const-specialized.rs:12:5 - | inside `FORTY_TWO` at $DIR/const-default-non-const-specialized.rs:35:5 -... -LL | const FORTY_TWO: u32 = - | -------------------- - | - = note: `#[deny(const_err)]` on by default - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #71800 - diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs index f6daba5595a8d..da6df064d4fd1 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs @@ -1,4 +1,4 @@ -// Tests that `T: ~const Foo` and `T: Foo` are treated as equivalent for the +// Tests that `T: Foo` and `T: ~const Foo` are treated as equivalent for the // purposes of min_specialization. // check-pass @@ -14,14 +14,14 @@ trait Foo {} trait Bar {} -impl const Bar for T +impl Bar for T where - T: ~const Foo, + T: Foo, {} -impl Bar for T +impl const Bar for T where - T: Foo, + T: ~const Foo, T: Specialize, {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs index cf6c292e8a465..84614f5459d63 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs @@ -1,6 +1,8 @@ // Tests that a non-const default impl can be specialized by a const trait impl, // but that the default impl cannot be used in a const context. +// run-pass + #![feature(const_trait_impl)] #![feature(min_specialization)] @@ -27,8 +29,10 @@ impl const Value for FortyTwo { } } -const ZERO: u32 = get_value::<()>(); //~ ERROR the trait bound `(): ~const Value` is not satisfied - -const FORTY_TWO: u32 = get_value::(); +fn main() { + let zero = get_value::<()>(); + assert_eq!(zero, 0); -fn main() {} + const FORTY_TWO: u32 = get_value::(); + assert_eq!(FORTY_TWO, 42); +} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr deleted file mode 100644 index 1065009c8910e..0000000000000 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.stderr +++ /dev/null @@ -1,20 +0,0 @@ -error[E0277]: the trait bound `(): ~const Value` is not satisfied - --> $DIR/non-const-default-const-specialized.rs:30:31 - | -LL | const ZERO: u32 = get_value::<()>(); - | ^^ the trait `~const Value` is not implemented for `()` - | -note: the trait `Value` is implemented for `()`, but that implementation is not `const` - --> $DIR/non-const-default-const-specialized.rs:30:31 - | -LL | const ZERO: u32 = get_value::<()>(); - | ^^ -note: required by a bound in `get_value` - --> $DIR/non-const-default-const-specialized.rs:11:23 - | -LL | const fn get_value() -> u32 { - | ^^^^^^^^^^^^ required by this bound in `get_value` - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0277`. From d492b9b000553f1407d9b75e15412d9df50314b2 Mon Sep 17 00:00:00 2001 From: Ben Reeves Date: Sat, 5 Nov 2022 23:12:57 -0500 Subject: [PATCH 30/38] Add #[const_trait] where needed in tests. --- .../specialization/const-default-const-specialized.rs | 1 + .../specialization/const-default-non-const-specialized.rs | 1 + .../specialization/const-default-non-const-specialized.stderr | 2 +- .../specialization/default-keyword.rs | 1 + .../specialization/issue-95186-specialize-on-tilde-const.rs | 3 +++ .../issue-95187-same-trait-bound-different-constness.rs | 2 ++ .../specialization/non-const-default-const-specialized.rs | 1 + 7 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.rs index 1eddfbf50f386..9ddea427cfd80 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-const-specialized.rs @@ -6,6 +6,7 @@ #![feature(const_trait_impl)] #![feature(min_specialization)] +#[const_trait] trait Value { fn value() -> u32; } diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs index a25329ba388d2..79dd4950af32a 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs @@ -4,6 +4,7 @@ #![feature(const_trait_impl)] #![feature(min_specialization)] +#[const_trait] trait Value { fn value() -> u32; } diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr index b0b76e7eca8ac..5232a8609cd80 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr @@ -1,5 +1,5 @@ error: cannot specialize on const impl with non-const impl - --> $DIR/const-default-non-const-specialized.rs:19:1 + --> $DIR/const-default-non-const-specialized.rs:20:1 | LL | impl Value for FortyTwo { | ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/default-keyword.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/default-keyword.rs index c03b0a0d19ca5..2aac0a2b4d111 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/default-keyword.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/default-keyword.rs @@ -3,6 +3,7 @@ #![feature(const_trait_impl)] #![feature(min_specialization)] +#[const_trait] trait Foo { fn foo(); } diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.rs index 1f7f47879d78b..9c2c2cf1610a2 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95186-specialize-on-tilde-const.rs @@ -6,9 +6,11 @@ #![feature(rustc_attrs)] #![feature(min_specialization)] +#[const_trait] #[rustc_specialization_trait] trait Specialize {} +#[const_trait] trait Foo {} impl const Foo for T {} @@ -18,6 +20,7 @@ where T: ~const Specialize, {} +#[const_trait] trait Bar {} impl const Bar for T diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs index da6df064d4fd1..3370aebf0634a 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs @@ -10,8 +10,10 @@ #[rustc_specialization_trait] trait Specialize {} +#[const_trait] trait Foo {} +#[const_trait] trait Bar {} impl Bar for T diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs index 84614f5459d63..35aa52fbd4ed2 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/non-const-default-const-specialized.rs @@ -6,6 +6,7 @@ #![feature(const_trait_impl)] #![feature(min_specialization)] +#[const_trait] trait Value { fn value() -> u32; } From c0ae62ee955712c96dec17170d4d63fd2b34f504 Mon Sep 17 00:00:00 2001 From: Ben Reeves Date: Sun, 6 Nov 2022 01:07:06 -0500 Subject: [PATCH 31/38] Require `~const` qualifier on trait bounds in specializing impls if present in base impl. --- .../src/impl_wf_check/min_specialization.rs | 59 ++++++++++++++----- ...fault-bound-non-const-specialized-bound.rs | 46 +++++++++++++++ ...t-bound-non-const-specialized-bound.stderr | 18 ++++++ ...efault-impl-non-const-specialized-impl.rs} | 3 +- ...lt-impl-non-const-specialized-impl.stderr} | 2 +- ...87-same-trait-bound-different-constness.rs | 19 +++++- .../specializing-constness.rs | 4 +- .../specializing-constness.stderr | 10 +++- 8 files changed, 137 insertions(+), 24 deletions(-) create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.rs create mode 100644 src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr rename src/test/ui/rfc-2632-const-trait-impl/specialization/{const-default-non-const-specialized.rs => const-default-impl-non-const-specialized-impl.rs} (81%) rename src/test/ui/rfc-2632-const-trait-impl/specialization/{const-default-non-const-specialized.stderr => const-default-impl-non-const-specialized-impl.stderr} (71%) diff --git a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs index 9c7150b79240a..f3cb558ef7093 100644 --- a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs +++ b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs @@ -391,7 +391,7 @@ fn check_predicates<'tcx>( ); for (predicate, span) in impl1_predicates { - if !impl2_predicates.iter().any(|pred2| trait_predicates_eq(predicate, *pred2)) { + if !impl2_predicates.iter().any(|pred2| trait_predicates_eq(tcx, predicate, *pred2, span)) { check_specialization_on(tcx, predicate, span) } } @@ -400,8 +400,8 @@ fn check_predicates<'tcx>( /// Checks if some predicate on the specializing impl (`predicate1`) is the same /// as some predicate on the base impl (`predicate2`). /// -/// This is slightly more complicated than simple syntactic equivalence, since -/// we want to equate `T: Tr` with `T: ~const Tr` so this can work: +/// This basically just checks syntactic equivalence, but is a little more +/// forgiving since we want to equate `T: Tr` with `T: ~const Tr` so this can work: /// /// ```ignore (illustrative) /// #[rustc_specialization_trait] @@ -410,27 +410,54 @@ fn check_predicates<'tcx>( /// impl Tr for T { } /// impl const Tr for T { } /// ``` +/// +/// However, we *don't* want to allow the reverse, i.e., when the bound on the +/// specializing impl is not as const as the bound on the base impl: +/// +/// ```ignore (illustrative) +/// impl const Tr for T { } +/// impl const Tr for T { } // should be T: ~const Bound +/// ``` +/// +/// So we make that check in this function and try to raise a helpful error message. fn trait_predicates_eq<'tcx>( + tcx: TyCtxt<'tcx>, predicate1: ty::Predicate<'tcx>, predicate2: ty::Predicate<'tcx>, + span: Span, ) -> bool { - let predicate_kind_without_constness = |kind: ty::PredicateKind<'tcx>| match kind { - ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _, polarity }) => { - ty::PredicateKind::Trait(ty::TraitPredicate { - trait_ref, - constness: ty::BoundConstness::NotConst, - polarity, - }) + let pred1_kind = predicate1.kind().no_bound_vars(); + let pred2_kind = predicate2.kind().no_bound_vars(); + let (trait_pred1, trait_pred2) = match (pred1_kind, pred2_kind) { + (Some(ty::PredicateKind::Trait(pred1)), Some(ty::PredicateKind::Trait(pred2))) => { + (pred1, pred2) } - _ => kind, + // Just use plain syntactic equivalence if either of the predicates aren't + // trait predicates or have bound vars. + _ => return pred1_kind == pred2_kind, + }; + + let predicates_equal_modulo_constness = { + let pred1_unconsted = + ty::TraitPredicate { constness: ty::BoundConstness::NotConst, ..trait_pred1 }; + let pred2_unconsted = + ty::TraitPredicate { constness: ty::BoundConstness::NotConst, ..trait_pred2 }; + pred1_unconsted == pred2_unconsted }; - // We rely on `check_constness` above to ensure that pred1 is const if pred2 - // is const. - let pred1_kind_not_const = predicate1.kind().map_bound(predicate_kind_without_constness); - let pred2_kind_not_const = predicate2.kind().map_bound(predicate_kind_without_constness); + if !predicates_equal_modulo_constness { + return false; + } + + // Check that the predicate on the specializing impl is at least as const as + // the one on the base. + if trait_pred2.constness == ty::BoundConstness::ConstIfConst + && trait_pred1.constness == ty::BoundConstness::NotConst + { + tcx.sess.struct_span_err(span, "missing `~const` qualifier").emit(); + } - pred1_kind_not_const == pred2_kind_not_const + true } #[instrument(level = "debug", skip(tcx))] diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.rs new file mode 100644 index 0000000000000..3ac909924864d --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.rs @@ -0,0 +1,46 @@ +// Tests that trait bounds on specializing trait impls must be `~const` if the +// same bound is present on the default impl and is `~const` there. + +#![feature(const_trait_impl)] +#![feature(rustc_attrs)] +#![feature(min_specialization)] + +#[rustc_specialization_trait] +trait Specialize {} + +#[const_trait] +trait Foo {} + +#[const_trait] +trait Bar {} + +// bgr360: I was only able to exercise the code path that raises the +// "missing ~const qualifier" error by making this base impl non-const, even +// though that doesn't really make sense to do. As seen below, if the base impl +// is made const, rustc fails earlier with an overlapping impl failure. +impl Bar for T +where + T: ~const Foo, +{} + +impl Bar for T +where + T: Foo, //~ ERROR missing `~const` qualifier + T: Specialize, +{} + +#[const_trait] +trait Baz {} + +impl const Baz for T +where + T: ~const Foo, +{} + +impl const Baz for T //~ ERROR conflicting implementations of trait `Baz` +where + T: Foo, + T: Specialize, +{} + +fn main() {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr new file mode 100644 index 0000000000000..583c4cec77fb6 --- /dev/null +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr @@ -0,0 +1,18 @@ +error: missing `~const` qualifier + --> $DIR/const-default-bound-non-const-specialized-bound.rs:28:8 + | +LL | T: Foo, + | ^^^ + +error[E0119]: conflicting implementations of trait `Baz` + --> $DIR/const-default-bound-non-const-specialized-bound.rs:40:1 + | +LL | impl const Baz for T + | ----------------------- first implementation here +... +LL | impl const Baz for T + | ^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0119`. diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-impl-non-const-specialized-impl.rs similarity index 81% rename from src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs rename to src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-impl-non-const-specialized-impl.rs index 79dd4950af32a..a3bb9b3f93eda 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-impl-non-const-specialized-impl.rs @@ -1,5 +1,4 @@ -// Tests that a const default trait impl cannot be specialized by a non-const -// trait impl. +// Tests that specializing trait impls must be at least as const as the default impl. #![feature(const_trait_impl)] #![feature(min_specialization)] diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-impl-non-const-specialized-impl.stderr similarity index 71% rename from src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr rename to src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-impl-non-const-specialized-impl.stderr index 5232a8609cd80..24766804708a3 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-non-const-specialized.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-impl-non-const-specialized-impl.stderr @@ -1,5 +1,5 @@ error: cannot specialize on const impl with non-const impl - --> $DIR/const-default-non-const-specialized.rs:20:1 + --> $DIR/const-default-impl-non-const-specialized-impl.rs:19:1 | LL | impl Value for FortyTwo { | ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs index 3370aebf0634a..1e6b1c6513b39 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/issue-95187-same-trait-bound-different-constness.rs @@ -1,5 +1,6 @@ -// Tests that `T: Foo` and `T: ~const Foo` are treated as equivalent for the -// purposes of min_specialization. +// Tests that `T: ~const Foo` in a specializing impl is treated as equivalent to +// `T: Foo` in the default impl for the purposes of specialization (i.e., it +// does not think that the user is attempting to specialize on trait `Foo`). // check-pass @@ -27,4 +28,18 @@ where T: Specialize, {} +#[const_trait] +trait Baz {} + +impl const Baz for T +where + T: Foo, +{} + +impl const Baz for T +where + T: ~const Foo, + T: Specialize, +{} + fn main() {} diff --git a/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.rs b/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.rs index ff0cd489d4744..9ab170f092006 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.rs +++ b/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.rs @@ -17,7 +17,9 @@ impl const A for T { } } -impl A for T { //~ ERROR: cannot specialize +impl A for T { +//~^ ERROR: cannot specialize +//~| ERROR: missing `~const` qualifier fn a() -> u32 { 3 } diff --git a/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.stderr b/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.stderr index 3296c109c4e73..281ba82d64429 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.stderr @@ -1,8 +1,14 @@ -error: cannot specialize on trait `Default` +error: cannot specialize on const impl with non-const impl + --> $DIR/specializing-constness.rs:20:1 + | +LL | impl A for T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing `~const` qualifier --> $DIR/specializing-constness.rs:20:9 | LL | impl A for T { | ^^^^^^^ -error: aborting due to previous error +error: aborting due to 2 previous errors From fe53cacff9f1e2aaeaeea4116d99cfa24d1f460f Mon Sep 17 00:00:00 2001 From: Ben Reeves Date: Thu, 10 Nov 2022 12:56:09 -0600 Subject: [PATCH 32/38] Apply PR feedback. --- .../src/impl_wf_check/min_specialization.rs | 19 +++++++++---------- ...t-bound-non-const-specialized-bound.stderr | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs index f3cb558ef7093..55cca0cd2d7b5 100644 --- a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs +++ b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs @@ -426,15 +426,13 @@ fn trait_predicates_eq<'tcx>( predicate2: ty::Predicate<'tcx>, span: Span, ) -> bool { - let pred1_kind = predicate1.kind().no_bound_vars(); - let pred2_kind = predicate2.kind().no_bound_vars(); + let pred1_kind = predicate1.kind().skip_binder(); + let pred2_kind = predicate2.kind().skip_binder(); let (trait_pred1, trait_pred2) = match (pred1_kind, pred2_kind) { - (Some(ty::PredicateKind::Trait(pred1)), Some(ty::PredicateKind::Trait(pred2))) => { - (pred1, pred2) - } + (ty::PredicateKind::Trait(pred1), ty::PredicateKind::Trait(pred2)) => (pred1, pred2), // Just use plain syntactic equivalence if either of the predicates aren't // trait predicates or have bound vars. - _ => return pred1_kind == pred2_kind, + _ => return predicate1 == predicate2, }; let predicates_equal_modulo_constness = { @@ -451,10 +449,11 @@ fn trait_predicates_eq<'tcx>( // Check that the predicate on the specializing impl is at least as const as // the one on the base. - if trait_pred2.constness == ty::BoundConstness::ConstIfConst - && trait_pred1.constness == ty::BoundConstness::NotConst - { - tcx.sess.struct_span_err(span, "missing `~const` qualifier").emit(); + match (trait_pred2.constness, trait_pred1.constness) { + (ty::BoundConstness::ConstIfConst, ty::BoundConstness::NotConst) => { + tcx.sess.struct_span_err(span, "missing `~const` qualifier for specialization").emit(); + } + _ => {} } true diff --git a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr index 583c4cec77fb6..4aea1979421c3 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/specialization/const-default-bound-non-const-specialized-bound.stderr @@ -1,4 +1,4 @@ -error: missing `~const` qualifier +error: missing `~const` qualifier for specialization --> $DIR/const-default-bound-non-const-specialized-bound.rs:28:8 | LL | T: Foo, From 0f2e45b18f47c9cb93267a82ed685f5d37f79367 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 29 Mar 2021 17:32:20 +0200 Subject: [PATCH 33/38] make `Sized` coinductive --- .../src/traits/select/mod.rs | 5 +++- src/test/ui/sized/coinductive-1.rs | 14 ++++++++++ src/test/ui/sized/coinductive-2.rs | 28 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/sized/coinductive-1.rs create mode 100644 src/test/ui/sized/coinductive-2.rs diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index de158a15d54b8..49bb3d03621f3 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -959,7 +959,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { fn coinductive_predicate(&self, predicate: ty::Predicate<'tcx>) -> bool { let result = match predicate.kind().skip_binder() { - ty::PredicateKind::Trait(ref data) => self.tcx().trait_is_auto(data.def_id()), + ty::PredicateKind::Trait(ref data) => { + self.tcx().trait_is_auto(data.def_id()) + || self.tcx().lang_items().sized_trait() == Some(data.def_id()) + } ty::PredicateKind::WellFormed(_) => true, _ => false, }; diff --git a/src/test/ui/sized/coinductive-1.rs b/src/test/ui/sized/coinductive-1.rs new file mode 100644 index 0000000000000..7bcd0f1fdaf6d --- /dev/null +++ b/src/test/ui/sized/coinductive-1.rs @@ -0,0 +1,14 @@ +// check-pass +struct Node>(C::Assoc); + +trait Trait { + type Assoc; +} + +impl Trait for Vec<()> { + type Assoc = Vec; +} + +fn main() { + let _ = Node::>(Vec::new()); +} diff --git a/src/test/ui/sized/coinductive-2.rs b/src/test/ui/sized/coinductive-2.rs new file mode 100644 index 0000000000000..212274d2e4b6c --- /dev/null +++ b/src/test/ui/sized/coinductive-2.rs @@ -0,0 +1,28 @@ +// run-pass +struct Node> { + _children: C::Collection, +} + +trait CollectionFactory { + type Collection; +} + +impl CollectionFactory for Vec<()> { + type Collection = Vec; +} + +trait Collection: Sized { + fn push(&mut self, v: T); +} + +impl Collection for Vec { + fn push(&mut self, v: T) { + self.push(v) + } +} + +fn main() { + let _ = Node::> { + _children: Vec::new(), + }; +} From 8ec6c84bb31f9b403b0b25bc4685ece5c744cb48 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 27 Apr 2021 14:24:19 +0200 Subject: [PATCH 34/38] add some more tests --- src/test/ui/sized/recursive-type-1.rs | 10 ++++++++++ src/test/ui/sized/recursive-type-2.rs | 13 +++++++++++++ src/test/ui/sized/recursive-type-2.stderr | 12 ++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 src/test/ui/sized/recursive-type-1.rs create mode 100644 src/test/ui/sized/recursive-type-2.rs create mode 100644 src/test/ui/sized/recursive-type-2.stderr diff --git a/src/test/ui/sized/recursive-type-1.rs b/src/test/ui/sized/recursive-type-1.rs new file mode 100644 index 0000000000000..cd6805967e524 --- /dev/null +++ b/src/test/ui/sized/recursive-type-1.rs @@ -0,0 +1,10 @@ +// check-pass +trait A { type Assoc; } + +impl A for () { + // FIXME: it would be nice for this to at least cause a warning. + type Assoc = Foo<()>; +} +struct Foo(T::Assoc); + +fn main() {} diff --git a/src/test/ui/sized/recursive-type-2.rs b/src/test/ui/sized/recursive-type-2.rs new file mode 100644 index 0000000000000..7d95417a6ffd9 --- /dev/null +++ b/src/test/ui/sized/recursive-type-2.rs @@ -0,0 +1,13 @@ +// build-fail +//~^ ERROR cycle detected when computing layout of `Foo<()>` + +trait A { type Assoc: ?Sized; } + +impl A for () { + type Assoc = Foo<()>; +} +struct Foo(T::Assoc); + +fn main() { + let x: Foo<()>; +} diff --git a/src/test/ui/sized/recursive-type-2.stderr b/src/test/ui/sized/recursive-type-2.stderr new file mode 100644 index 0000000000000..2102c934da36f --- /dev/null +++ b/src/test/ui/sized/recursive-type-2.stderr @@ -0,0 +1,12 @@ +error[E0391]: cycle detected when computing layout of `Foo<()>` + | + = note: ...which again requires computing layout of `Foo<()>`, completing the cycle +note: cycle used when optimizing MIR for `main` + --> $DIR/recursive-type-2.rs:11:1 + | +LL | fn main() { + | ^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0391`. From 43ad19b2506c4e4ce6204e3ecf8b35869bd76eff Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 10 Aug 2022 16:47:26 +0000 Subject: [PATCH 35/38] bless tests --- .../generic-associated-types/bugs/issue-80626.rs | 7 ++----- .../bugs/issue-80626.stderr | 15 --------------- .../ui/generic-associated-types/issue-87750.rs | 8 ++++++-- .../generic-associated-types/issue-87750.stderr | 9 --------- .../projection-bound-cycle-generic.rs | 2 +- .../projection-bound-cycle-generic.stderr | 14 ++++---------- .../projection-bound-cycle.rs | 2 +- .../projection-bound-cycle.stderr | 14 ++++---------- src/test/ui/sized/recursive-type-2.stderr | 3 ++- src/test/ui/traits/issue-82830.rs | 4 +++- src/test/ui/traits/issue-82830.stderr | 15 --------------- 11 files changed, 23 insertions(+), 70 deletions(-) delete mode 100644 src/test/ui/generic-associated-types/bugs/issue-80626.stderr delete mode 100644 src/test/ui/generic-associated-types/issue-87750.stderr delete mode 100644 src/test/ui/traits/issue-82830.stderr diff --git a/src/test/ui/generic-associated-types/bugs/issue-80626.rs b/src/test/ui/generic-associated-types/bugs/issue-80626.rs index f6aa6b36e13dc..d6e18010f3b27 100644 --- a/src/test/ui/generic-associated-types/bugs/issue-80626.rs +++ b/src/test/ui/generic-associated-types/bugs/issue-80626.rs @@ -1,7 +1,4 @@ -// check-fail -// known-bug: #80626 - -// This should pass, but it requires `Sized` to be coinductive. +// check-pass trait Allocator { type Allocated; @@ -9,7 +6,7 @@ trait Allocator { enum LinkedList { Head, - Next(A::Allocated) + Next(A::Allocated), } fn main() {} diff --git a/src/test/ui/generic-associated-types/bugs/issue-80626.stderr b/src/test/ui/generic-associated-types/bugs/issue-80626.stderr deleted file mode 100644 index 9a0f332ed4736..0000000000000 --- a/src/test/ui/generic-associated-types/bugs/issue-80626.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0275]: overflow evaluating the requirement `LinkedList: Sized` - --> $DIR/issue-80626.rs:12:10 - | -LL | Next(A::Allocated) - | ^^^^^^^^^^^^^^^^^^ - | -note: required by a bound in `Allocator::Allocated` - --> $DIR/issue-80626.rs:7:20 - | -LL | type Allocated; - | ^ required by this bound in `Allocator::Allocated` - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0275`. diff --git a/src/test/ui/generic-associated-types/issue-87750.rs b/src/test/ui/generic-associated-types/issue-87750.rs index 0a11a0f3ae0e4..b35657989efb9 100644 --- a/src/test/ui/generic-associated-types/issue-87750.rs +++ b/src/test/ui/generic-associated-types/issue-87750.rs @@ -1,3 +1,5 @@ +// check-pass + trait PointerFamily { type Pointer; } @@ -10,11 +12,13 @@ impl PointerFamily for RcFamily { } #[allow(dead_code)] -enum Node where P::Pointer>: Sized { +enum Node +where + P::Pointer>: Sized, +{ Cons(P::Pointer>), } fn main() { let _list: ::Pointer>; - //~^ ERROR overflow evaluating the requirement `Node: Sized` } diff --git a/src/test/ui/generic-associated-types/issue-87750.stderr b/src/test/ui/generic-associated-types/issue-87750.stderr deleted file mode 100644 index b358ca273ca79..0000000000000 --- a/src/test/ui/generic-associated-types/issue-87750.stderr +++ /dev/null @@ -1,9 +0,0 @@ -error[E0275]: overflow evaluating the requirement `Node: Sized` - --> $DIR/issue-87750.rs:18:16 - | -LL | let _list: ::Pointer>; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0275`. diff --git a/src/test/ui/generic-associated-types/projection-bound-cycle-generic.rs b/src/test/ui/generic-associated-types/projection-bound-cycle-generic.rs index 58d57df63c1bd..ecf6f69c9fa7e 100644 --- a/src/test/ui/generic-associated-types/projection-bound-cycle-generic.rs +++ b/src/test/ui/generic-associated-types/projection-bound-cycle-generic.rs @@ -21,6 +21,7 @@ impl Foo for Number { // ``` // which it is :) type Item = [T] where [T]: Sized; + //~^ ERROR overflow evaluating the requirement ` as Foo>::Item == _` } struct OnlySized where T: Sized { f: T } @@ -40,7 +41,6 @@ impl Bar for T where T: Foo { // can use the bound on `Foo::Item` for this, but that requires // `wf(::Item)`, which is an invalid cycle. type Assoc = OnlySized<::Item>; - //~^ ERROR overflow evaluating the requirement `::Item: Sized` } fn foo() { diff --git a/src/test/ui/generic-associated-types/projection-bound-cycle-generic.stderr b/src/test/ui/generic-associated-types/projection-bound-cycle-generic.stderr index 27c1a82994a53..aae9a56bb6128 100644 --- a/src/test/ui/generic-associated-types/projection-bound-cycle-generic.stderr +++ b/src/test/ui/generic-associated-types/projection-bound-cycle-generic.stderr @@ -1,14 +1,8 @@ -error[E0275]: overflow evaluating the requirement `::Item: Sized` - --> $DIR/projection-bound-cycle-generic.rs:42:18 +error[E0275]: overflow evaluating the requirement ` as Foo>::Item == _` + --> $DIR/projection-bound-cycle-generic.rs:23:5 | -LL | type Assoc = OnlySized<::Item>; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: required by a bound in `OnlySized` - --> $DIR/projection-bound-cycle-generic.rs:26:18 - | -LL | struct OnlySized where T: Sized { f: T } - | ^ required by this bound in `OnlySized` +LL | type Item = [T] where [T]: Sized; + | ^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/generic-associated-types/projection-bound-cycle.rs b/src/test/ui/generic-associated-types/projection-bound-cycle.rs index 4cad1f61319ef..b51ae7ef20186 100644 --- a/src/test/ui/generic-associated-types/projection-bound-cycle.rs +++ b/src/test/ui/generic-associated-types/projection-bound-cycle.rs @@ -24,6 +24,7 @@ impl Foo for Number { // ``` // which it is :) type Item = str where str: Sized; + //~^ ERROR overflow evaluating the requirement `::Item == _` } struct OnlySized where T: Sized { f: T } @@ -43,7 +44,6 @@ impl Bar for T where T: Foo { // can use the bound on `Foo::Item` for this, but that requires // `wf(::Item)`, which is an invalid cycle. type Assoc = OnlySized<::Item>; - //~^ ERROR overflow evaluating the requirement `::Item: Sized` } fn foo() { diff --git a/src/test/ui/generic-associated-types/projection-bound-cycle.stderr b/src/test/ui/generic-associated-types/projection-bound-cycle.stderr index a46518c80da76..b1b8afeecd02f 100644 --- a/src/test/ui/generic-associated-types/projection-bound-cycle.stderr +++ b/src/test/ui/generic-associated-types/projection-bound-cycle.stderr @@ -1,14 +1,8 @@ -error[E0275]: overflow evaluating the requirement `::Item: Sized` - --> $DIR/projection-bound-cycle.rs:45:18 +error[E0275]: overflow evaluating the requirement `::Item == _` + --> $DIR/projection-bound-cycle.rs:26:5 | -LL | type Assoc = OnlySized<::Item>; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: required by a bound in `OnlySized` - --> $DIR/projection-bound-cycle.rs:29:18 - | -LL | struct OnlySized where T: Sized { f: T } - | ^ required by this bound in `OnlySized` +LL | type Item = str where str: Sized; + | ^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/sized/recursive-type-2.stderr b/src/test/ui/sized/recursive-type-2.stderr index 2102c934da36f..d0e6e9db07e9b 100644 --- a/src/test/ui/sized/recursive-type-2.stderr +++ b/src/test/ui/sized/recursive-type-2.stderr @@ -1,7 +1,8 @@ error[E0391]: cycle detected when computing layout of `Foo<()>` | + = note: ...which requires computing layout of `<() as A>::Assoc`... = note: ...which again requires computing layout of `Foo<()>`, completing the cycle -note: cycle used when optimizing MIR for `main` +note: cycle used when elaborating drops for `main` --> $DIR/recursive-type-2.rs:11:1 | LL | fn main() { diff --git a/src/test/ui/traits/issue-82830.rs b/src/test/ui/traits/issue-82830.rs index c8289b2e30b4d..37bae2e90a595 100644 --- a/src/test/ui/traits/issue-82830.rs +++ b/src/test/ui/traits/issue-82830.rs @@ -1,10 +1,12 @@ +// check-pass + trait A { type B; } type MaybeBox = >>::B; struct P { - t: MaybeBox

, //~ ERROR: overflow evaluating the requirement `P: Sized` + t: MaybeBox

, } impl A for P { diff --git a/src/test/ui/traits/issue-82830.stderr b/src/test/ui/traits/issue-82830.stderr deleted file mode 100644 index 6a597a402156f..0000000000000 --- a/src/test/ui/traits/issue-82830.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error[E0275]: overflow evaluating the requirement `P: Sized` - --> $DIR/issue-82830.rs:7:8 - | -LL | t: MaybeBox

, - | ^^^^^^^^^^^ - | -note: required for `P` to implement `A>` - --> $DIR/issue-82830.rs:10:12 - | -LL | impl A for P { - | ^^^^^^^ ^ - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0275`. From fea8d0eb999dbe732a70cd61518d79195c139b2c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 10 Nov 2022 21:29:20 +0000 Subject: [PATCH 36/38] More nits --- compiler/rustc_middle/src/ty/mod.rs | 4 ++++ .../rustc_trait_selection/src/traits/select/mod.rs | 5 +---- src/test/ui/sized/coinductive-1-gat.rs | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/sized/coinductive-1-gat.rs diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index b509ae6dd3b85..18eb06b83c9df 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -2506,6 +2506,10 @@ impl<'tcx> TyCtxt<'tcx> { self.trait_def(trait_def_id).has_auto_impl } + pub fn trait_is_coinductive(self, trait_def_id: DefId) -> bool { + self.trait_is_auto(trait_def_id) || self.lang_items().sized_trait() == Some(trait_def_id) + } + /// Returns layout of a generator. Layout might be unavailable if the /// generator is tainted by errors. pub fn generator_layout(self, def_id: DefId) -> Option<&'tcx GeneratorLayout<'tcx>> { diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 49bb3d03621f3..a12f67125bbc0 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -959,10 +959,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { fn coinductive_predicate(&self, predicate: ty::Predicate<'tcx>) -> bool { let result = match predicate.kind().skip_binder() { - ty::PredicateKind::Trait(ref data) => { - self.tcx().trait_is_auto(data.def_id()) - || self.tcx().lang_items().sized_trait() == Some(data.def_id()) - } + ty::PredicateKind::Trait(ref data) => self.tcx().trait_is_coinductive(data.def_id()), ty::PredicateKind::WellFormed(_) => true, _ => false, }; diff --git a/src/test/ui/sized/coinductive-1-gat.rs b/src/test/ui/sized/coinductive-1-gat.rs new file mode 100644 index 0000000000000..cdf70920f0095 --- /dev/null +++ b/src/test/ui/sized/coinductive-1-gat.rs @@ -0,0 +1,14 @@ +// check-pass +struct Node(C::Assoc::); + +trait Trait { + type Assoc; +} + +impl Trait for Vec<()> { + type Assoc = Vec; +} + +fn main() { + let _ = Node::>(Vec::new()); +} From 94f67e667be3efd1845bb95fcd25fcce11cf983c Mon Sep 17 00:00:00 2001 From: Ben Reeves Date: Thu, 10 Nov 2022 22:14:08 -0600 Subject: [PATCH 37/38] Oops, bless this test. --- .../ui/rfc-2632-const-trait-impl/specializing-constness.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.stderr b/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.stderr index 281ba82d64429..843fc6ce84d45 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/specializing-constness.stderr @@ -4,7 +4,7 @@ error: cannot specialize on const impl with non-const impl LL | impl A for T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: missing `~const` qualifier +error: missing `~const` qualifier for specialization --> $DIR/specializing-constness.rs:20:9 | LL | impl A for T { From 05824cd7b7f9b022a68f49b676a58088b6b85118 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 11 Nov 2022 07:35:09 -0700 Subject: [PATCH 38/38] rustdoc: fix HTML validation failure by escaping `data-ty` --- src/librustdoc/html/render/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index fffd90c51fad1..266ec2ac7ad73 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1312,10 +1312,10 @@ pub(crate) fn notable_traits_button(ty: &clean::Type, cx: &mut Context<'_>) -> O if has_notable_trait { cx.types_with_notable_traits.insert(ty.clone()); Some(format!( - "\ + "\ \ ", - ty = ty.print(cx), + ty = Escape(&format!("{:#}", ty.print(cx))), )) } else { None