From 9b5a974bd5c398e5706e463045121b20f0f6abb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 22 Jul 2020 12:01:56 -0700 Subject: [PATCH 01/13] Correctly parse `{} && false` in tail expression Fix #74233. --- src/librustc_ast/util/parser.rs | 1 - src/librustc_parse/parser/expr.rs | 11 ++++++-- src/librustc_typeck/check/demand.rs | 1 + src/librustc_typeck/check/mod.rs | 8 ++++++ src/test/ui/parser/expr-as-stmt-2.rs | 10 +++++++ src/test/ui/parser/expr-as-stmt-2.stderr | 33 ++++++++++++++++++++++++ src/test/ui/parser/expr-as-stmt.fixed | 6 ----- src/test/ui/parser/expr-as-stmt.rs | 6 ----- src/test/ui/parser/expr-as-stmt.stderr | 12 ++------- 9 files changed, 63 insertions(+), 25 deletions(-) create mode 100644 src/test/ui/parser/expr-as-stmt-2.rs create mode 100644 src/test/ui/parser/expr-as-stmt-2.stderr diff --git a/src/librustc_ast/util/parser.rs b/src/librustc_ast/util/parser.rs index e5bcc571d4176..2ee94965756a5 100644 --- a/src/librustc_ast/util/parser.rs +++ b/src/librustc_ast/util/parser.rs @@ -222,7 +222,6 @@ impl AssocOp { Greater | // `{ 42 } > 3` GreaterEqual | // `{ 42 } >= 3` AssignOp(_) | // `{ 42 } +=` - LAnd | // `{ 42 } &&foo` As | // `{ 42 } as usize` // Equal | // `{ 42 } == { 42 }` Accepting these here would regress incorrect // NotEqual | // `{ 42 } != { 42 } struct literals parser recovery. diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index 3926122606e6d..223986635a03a 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -295,11 +295,18 @@ impl<'a> Parser<'a> { // want to keep their span info to improve diagnostics in these cases in a later stage. (true, Some(AssocOp::Multiply)) | // `{ 42 } *foo = bar;` or `{ 42 } * 3` (true, Some(AssocOp::Subtract)) | // `{ 42 } -5` - (true, Some(AssocOp::LAnd)) | // `{ 42 } &&x` (#61475) (true, Some(AssocOp::Add)) // `{ 42 } + 42 // If the next token is a keyword, then the tokens above *are* unambiguously incorrect: // `if x { a } else { b } && if y { c } else { d }` - if !self.look_ahead(1, |t| t.is_reserved_ident()) => { + if !self.look_ahead(1, |t| t.is_used_keyword()) => { + // These cases are ambiguous and can't be identified in the parser alone. + let sp = self.sess.source_map().start_point(self.token.span); + self.sess.ambiguous_block_expr_parse.borrow_mut().insert(sp, lhs.span); + false + } + (true, Some(AssocOp::LAnd)) => { + // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }`. Separated from the + // above due to #74233. // These cases are ambiguous and can't be identified in the parser alone. let sp = self.sess.source_map().start_point(self.token.span); self.sess.ambiguous_block_expr_parse.borrow_mut().insert(sp, lhs.span); diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index be83ab259c2ec..46303a99278dd 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -34,6 +34,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty); self.suggest_missing_await(err, expr, expected, expr_ty); + self.suggest_missing_parentheses(err, expr); self.note_need_for_fn_pointer(err, expected, expr_ty); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 04e02704296de..57de1780c65ea 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5403,6 +5403,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + fn suggest_missing_parentheses(&self, err: &mut DiagnosticBuilder<'_>, expr: &hir::Expr<'_>) { + let sp = self.tcx.sess.source_map().start_point(expr.span); + if let Some(sp) = self.tcx.sess.parse_sess.ambiguous_block_expr_parse.borrow().get(&sp) { + // `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }` + self.tcx.sess.parse_sess.expr_parentheses_needed(err, *sp, None); + } + } + fn note_need_for_fn_pointer( &self, err: &mut DiagnosticBuilder<'_>, diff --git a/src/test/ui/parser/expr-as-stmt-2.rs b/src/test/ui/parser/expr-as-stmt-2.rs new file mode 100644 index 0000000000000..3a18bdc3b730a --- /dev/null +++ b/src/test/ui/parser/expr-as-stmt-2.rs @@ -0,0 +1,10 @@ +// This is not autofixable because we give extra suggestions to end the first expression with `;`. +fn foo(a: Option, b: Option) -> bool { + if let Some(x) = a { true } else { false } + //~^ ERROR mismatched types + //~| ERROR mismatched types + && //~ ERROR mismatched types + if let Some(y) = a { true } else { false } +} + +fn main() {} diff --git a/src/test/ui/parser/expr-as-stmt-2.stderr b/src/test/ui/parser/expr-as-stmt-2.stderr new file mode 100644 index 0000000000000..ee07c36763356 --- /dev/null +++ b/src/test/ui/parser/expr-as-stmt-2.stderr @@ -0,0 +1,33 @@ +error[E0308]: mismatched types + --> $DIR/expr-as-stmt-2.rs:3:26 + | +LL | if let Some(x) = a { true } else { false } + | ---------------------^^^^------------------ help: consider using a semicolon here + | | | + | | expected `()`, found `bool` + | expected this to be `()` + +error[E0308]: mismatched types + --> $DIR/expr-as-stmt-2.rs:3:40 + | +LL | if let Some(x) = a { true } else { false } + | -----------------------------------^^^^^--- help: consider using a semicolon here + | | | + | | expected `()`, found `bool` + | expected this to be `()` + +error[E0308]: mismatched types + --> $DIR/expr-as-stmt-2.rs:6:5 + | +LL | fn foo(a: Option, b: Option) -> bool { + | ---- expected `bool` because of return type +LL | if let Some(x) = a { true } else { false } + | ------------------------------------------ help: parentheses are required to parse this as an expression: `(if let Some(x) = a { true } else { false })` +... +LL | / && +LL | | if let Some(y) = a { true } else { false } + | |______________________________________________^ expected `bool`, found `&&bool` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/parser/expr-as-stmt.fixed b/src/test/ui/parser/expr-as-stmt.fixed index 1ce6f9c25034f..02816ef2791b0 100644 --- a/src/test/ui/parser/expr-as-stmt.fixed +++ b/src/test/ui/parser/expr-as-stmt.fixed @@ -25,12 +25,6 @@ fn baz() -> i32 { //~^ ERROR mismatched types } -fn qux(a: Option, b: Option) -> bool { - (if let Some(x) = a { true } else { false }) - && //~ ERROR expected expression - if let Some(y) = a { true } else { false } -} - fn moo(x: u32) -> bool { (match x { _ => 1, diff --git a/src/test/ui/parser/expr-as-stmt.rs b/src/test/ui/parser/expr-as-stmt.rs index b526c17488eaf..93baa8278f890 100644 --- a/src/test/ui/parser/expr-as-stmt.rs +++ b/src/test/ui/parser/expr-as-stmt.rs @@ -25,12 +25,6 @@ fn baz() -> i32 { //~^ ERROR mismatched types } -fn qux(a: Option, b: Option) -> bool { - if let Some(x) = a { true } else { false } - && //~ ERROR expected expression - if let Some(y) = a { true } else { false } -} - fn moo(x: u32) -> bool { match x { _ => 1, diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr index 4d93e130901e7..324aed0ad7cf6 100644 --- a/src/test/ui/parser/expr-as-stmt.stderr +++ b/src/test/ui/parser/expr-as-stmt.stderr @@ -22,16 +22,8 @@ LL | { 42 } + foo; | | | help: parentheses are required to parse this as an expression: `({ 42 })` -error: expected expression, found `&&` - --> $DIR/expr-as-stmt.rs:30:5 - | -LL | if let Some(x) = a { true } else { false } - | ------------------------------------------ help: parentheses are required to parse this as an expression: `(if let Some(x) = a { true } else { false })` -LL | && - | ^^ expected expression - error: expected expression, found `>` - --> $DIR/expr-as-stmt.rs:37:7 + --> $DIR/expr-as-stmt.rs:31:7 | LL | } > 0 | ^ expected expression @@ -75,7 +67,7 @@ LL | { 3 } * 3 | | | help: parentheses are required to parse this as an expression: `({ 3 })` -error: aborting due to 10 previous errors +error: aborting due to 9 previous errors Some errors have detailed explanations: E0308, E0614. For more information about an error, try `rustc --explain E0308`. From 0a4f4e87e04b88d5269003d7868f33cd818e1ba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 8 Aug 2020 20:11:16 -0700 Subject: [PATCH 02/13] Fix ICE #75307 in `format` Remove usages of `unwrap` (even when some are safe today). --- src/librustc_builtin_macros/format.rs | 33 +++++++++++---------------- src/test/ui/issues/issue-75307.rs | 3 +++ src/test/ui/issues/issue-75307.stderr | 10 ++++++++ 3 files changed, 26 insertions(+), 20 deletions(-) create mode 100644 src/test/ui/issues/issue-75307.rs create mode 100644 src/test/ui/issues/issue-75307.stderr diff --git a/src/librustc_builtin_macros/format.rs b/src/librustc_builtin_macros/format.rs index 55eab24b8a510..78cead02b7b7c 100644 --- a/src/librustc_builtin_macros/format.rs +++ b/src/librustc_builtin_macros/format.rs @@ -149,7 +149,7 @@ fn parse_args<'a>( return Err(err); } else { // ...after that delegate to `expect` to also include the other expected tokens. - return Err(p.expect(&token::Comma).err().unwrap()); + let _ = p.expect(&token::Comma)?; } } first = false; @@ -359,24 +359,18 @@ impl<'a, 'b> Context<'a, 'b> { // for `println!("{7:7$}", 1);` refs.sort(); refs.dedup(); - let (arg_list, mut sp) = if refs.len() == 1 { - let spans: Vec<_> = spans.into_iter().filter_map(|sp| sp.copied()).collect(); - ( - format!("argument {}", refs[0]), - if spans.is_empty() { - MultiSpan::from_span(self.fmtsp) - } else { - MultiSpan::from_spans(spans) - }, - ) + let spans: Vec<_> = spans.into_iter().filter_map(|sp| sp.copied()).collect(); + let sp = if self.arg_spans.is_empty() || spans.is_empty() { + MultiSpan::from_span(self.fmtsp) + } else { + MultiSpan::from_spans(spans) + }; + let arg_list = if refs.len() == 1 { + format!("argument {}", refs[0]) } else { - let pos = MultiSpan::from_spans(spans.into_iter().map(|s| *s.unwrap()).collect()); let reg = refs.pop().unwrap(); - (format!("arguments {head} and {tail}", head = refs.join(", "), tail = reg,), pos) + format!("arguments {head} and {tail}", head = refs.join(", "), tail = reg) }; - if self.arg_spans.is_empty() { - sp = MultiSpan::from_span(self.fmtsp); - } e = self.ecx.struct_span_err( sp, @@ -1067,10 +1061,9 @@ pub fn expand_preparsed_format_args( let args_unused = errs_len; let mut diag = { - if errs_len == 1 { - let (sp, msg) = errs.into_iter().next().unwrap(); - let mut diag = cx.ecx.struct_span_err(sp, msg); - diag.span_label(sp, msg); + if let [(sp, msg)] = &errs[..] { + let mut diag = cx.ecx.struct_span_err(*sp, *msg); + diag.span_label(*sp, *msg); diag } else { let mut diag = cx.ecx.struct_span_err( diff --git a/src/test/ui/issues/issue-75307.rs b/src/test/ui/issues/issue-75307.rs new file mode 100644 index 0000000000000..2fe112a3b95d4 --- /dev/null +++ b/src/test/ui/issues/issue-75307.rs @@ -0,0 +1,3 @@ +fn main() { + format!(r"{}{}{}", named_arg=1); //~ ERROR invalid reference to positional arguments 1 and 2 +} diff --git a/src/test/ui/issues/issue-75307.stderr b/src/test/ui/issues/issue-75307.stderr new file mode 100644 index 0000000000000..4a5d997e00d74 --- /dev/null +++ b/src/test/ui/issues/issue-75307.stderr @@ -0,0 +1,10 @@ +error: invalid reference to positional arguments 1 and 2 (there is 1 argument) + --> $DIR/issue-75307.rs:2:13 + | +LL | format!(r"{}{}{}", named_arg=1); + | ^^^^^^^^^ + | + = note: positional arguments are zero-based + +error: aborting due to previous error + From 0356bb9fbba55f0a8fbe38731d41f57048f2e00b Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum Date: Tue, 11 Aug 2020 14:07:08 -0400 Subject: [PATCH 03/13] Revert "Suppress debuginfo on naked function arguments" This reverts commit 25670749b44a9c7a4cfd3fbf780bbe3344a9a6c5. This commit does not actually fix the problem. It merely removes the name of the argument from the LLVM output. Even without the name, Rust codegen still spills the (nameless) variable onto the stack which is the root cause. The root cause is solved in the next commit. --- src/librustc_mir_build/build/mod.rs | 12 +----- src/test/codegen/naked-functions.rs | 4 +- .../debuginfo/function-arguments-naked.rs | 42 ------------------- 3 files changed, 3 insertions(+), 55 deletions(-) delete mode 100644 src/test/debuginfo/function-arguments-naked.rs diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index f3f3c3e33a46d..215a0c7dfdf27 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -10,7 +10,6 @@ use rustc_hir::lang_items; use rustc_hir::{GeneratorKind, HirIdMap, Node}; use rustc_index::vec::{Idx, IndexVec}; use rustc_infer::infer::TyCtxtInferExt; -use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::region; use rustc_middle::mir::*; use rustc_middle::ty::subst::Subst; @@ -798,22 +797,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { argument_scope: region::Scope, ast_body: &'tcx hir::Expr<'tcx>, ) -> BlockAnd<()> { - let tcx = self.hir.tcx(); - let attrs = tcx.codegen_fn_attrs(fn_def_id); - let naked = attrs.flags.contains(CodegenFnAttrFlags::NAKED); - // Allocate locals for the function arguments for &ArgInfo(ty, _, arg_opt, _) in arguments.iter() { let source_info = SourceInfo::outermost(arg_opt.map_or(self.fn_span, |arg| arg.pat.span)); let arg_local = self.local_decls.push(LocalDecl::with_source_info(ty, source_info)); - // Emit function argument debuginfo only for non-naked functions. - // See: https://github.com/rust-lang/rust/issues/42779 - if naked { - continue; - } - // If this is a simple binding pattern, give debuginfo a nice name. if let Some(arg) = arg_opt { if let Some(ident) = arg.pat.simple_ident() { @@ -826,6 +815,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + let tcx = self.hir.tcx(); let tcx_hir = tcx.hir(); let hir_typeck_results = self.hir.typeck_results(); diff --git a/src/test/codegen/naked-functions.rs b/src/test/codegen/naked-functions.rs index 758c6c4da9293..493c1b9f0ba6b 100644 --- a/src/test/codegen/naked-functions.rs +++ b/src/test/codegen/naked-functions.rs @@ -18,7 +18,7 @@ pub fn naked_empty() { // CHECK-NEXT: define void @naked_with_args(i{{[0-9]+( %0)?}}) pub fn naked_with_args(a: isize) { // CHECK-NEXT: {{.+}}: - // CHECK-NEXT: %_1 = alloca i{{[0-9]+}} + // CHECK-NEXT: %a = alloca i{{[0-9]+}} &a; // keep variable in an alloca // CHECK: ret void } @@ -39,7 +39,7 @@ pub fn naked_with_return() -> isize { #[naked] pub fn naked_with_args_and_return(a: isize) -> isize { // CHECK-NEXT: {{.+}}: - // CHECK-NEXT: %_1 = alloca i{{[0-9]+}} + // CHECK-NEXT: %a = alloca i{{[0-9]+}} &a; // keep variable in an alloca // CHECK: ret i{{[0-9]+}} %{{[0-9]+}} a diff --git a/src/test/debuginfo/function-arguments-naked.rs b/src/test/debuginfo/function-arguments-naked.rs deleted file mode 100644 index 5f3a1eb44e4e5..0000000000000 --- a/src/test/debuginfo/function-arguments-naked.rs +++ /dev/null @@ -1,42 +0,0 @@ -// min-lldb-version: 310 - -// We have to ignore android because of this issue: -// https://github.com/rust-lang/rust/issues/74847 -// ignore-android -// -// We need to use inline assembly, so just use one platform -// only-x86_64 - -// compile-flags:-g - -// === GDB TESTS =================================================================================== - -// gdb-command:run - -// gdb-command:info args -// gdb-check:No arguments. -// gdb-command:continue - -// === LLDB TESTS ================================================================================== - -// lldb-command:run - -// lldb-command:frame variable -// lldbg-check:(unsigned long) = 111 (unsigned long) = 222 -// lldbr-check:(unsigned long) = 111 (unsigned long) = 222 -// lldb-command:continue - - -#![feature(asm)] -#![feature(naked_functions)] -#![feature(omit_gdb_pretty_printer_section)] -#![omit_gdb_pretty_printer_section] - -fn main() { - naked(111, 222); -} - -#[naked] -extern "C" fn naked(x: usize, y: usize) { - unsafe { asm!("ret"); } // #break -} From 050fb380aaa5f95ca27d6365cea06ea614c4cbf7 Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum Date: Tue, 11 Aug 2020 14:09:39 -0400 Subject: [PATCH 04/13] Don't spill operands onto the stack in naked functions Currently, the code spills operands onto the stack for the purpose of debuginfo. However, naked functions can only contain an asm block. Therefore, attempting to spill the operands on the stack is undefined behavior. Fixes https://github.com/rust-lang/rust/issues/42779 cc https://github.com/rust-lang/rust/issues/32408 --- src/librustc_codegen_ssa/mir/debuginfo.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/librustc_codegen_ssa/mir/debuginfo.rs b/src/librustc_codegen_ssa/mir/debuginfo.rs index d166a27b5a982..d8a530d98faa7 100644 --- a/src/librustc_codegen_ssa/mir/debuginfo.rs +++ b/src/librustc_codegen_ssa/mir/debuginfo.rs @@ -1,6 +1,7 @@ use crate::traits::*; use rustc_hir::def_id::CrateNum; use rustc_index::vec::IndexVec; +use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir; use rustc_middle::ty; use rustc_session::config::DebugInfo; @@ -216,6 +217,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { LocalRef::Operand(None) => return, LocalRef::Operand(Some(operand)) => { + // Don't spill operands onto the stack in naked functions. + // See: https://github.com/rust-lang/rust/issues/42779 + let attrs = bx.tcx().codegen_fn_attrs(self.instance.def_id()); + if attrs.flags.contains(CodegenFnAttrFlags::NAKED) { + return; + } + // "Spill" the value onto the stack, for debuginfo, // without forcing non-debuginfo uses of the local // to also load from the stack every single time. From 0e26f9b994997e2c8256e04de66812e3db6cf37e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 12 Aug 2020 19:37:08 +0200 Subject: [PATCH 05/13] fix LocalInfo doc comment --- src/librustc_middle/mir/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index 3b0c480f6d400..98fd68927f1c2 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -857,9 +857,12 @@ pub struct LocalDecl<'tcx> { #[cfg(target_arch = "x86_64")] static_assert_size!(LocalDecl<'_>, 56); -/// Extra information about a some locals that's used for diagnostics. (Not -/// used for non-StaticRef temporaries, the return place, or anonymous function -/// parameters.) +/// Extra information about a some locals that's used for diagnostics and for +/// classifying variables into local variables, statics, etc, which is needed e.g. +/// for unsafety checking. +/// +/// Not used for non-StaticRef temporaries, the return place, or anonymous +/// function parameters. #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] pub enum LocalInfo<'tcx> { /// A user-defined local variable or function parameter From bff104d4db7ce785dc931a58d88d1dea891efecb Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 12 Aug 2020 16:00:44 -0400 Subject: [PATCH 06/13] Remove unused tcx parameter --- src/librustc_query_system/query/caches.rs | 9 +++------ src/librustc_query_system/query/plumbing.rs | 10 +++++----- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/librustc_query_system/query/caches.rs b/src/librustc_query_system/query/caches.rs index f0beec0a17726..1839e1af45eef 100644 --- a/src/librustc_query_system/query/caches.rs +++ b/src/librustc_query_system/query/caches.rs @@ -43,9 +43,8 @@ pub trait QueryCache: QueryStorage { OnHit: FnOnce(&Self::Stored, DepNodeIndex) -> R, OnMiss: FnOnce(Self::Key, QueryLookup<'_, CTX, Self::Key, Self::Sharded>) -> R; - fn complete( + fn complete( &self, - tcx: CTX, lock_sharded_storage: &mut Self::Sharded, key: Self::Key, value: Self::Value, @@ -112,9 +111,8 @@ impl QueryCache for DefaultCache { } #[inline] - fn complete( + fn complete( &self, - _: CTX, lock_sharded_storage: &mut Self::Sharded, key: K, value: V, @@ -195,9 +193,8 @@ impl<'tcx, K: Eq + Hash, V: 'tcx> QueryCache for ArenaCache<'tcx, K, V> { } #[inline] - fn complete( + fn complete( &self, - _: CTX, lock_sharded_storage: &mut Self::Sharded, key: K, value: V, diff --git a/src/librustc_query_system/query/plumbing.rs b/src/librustc_query_system/query/plumbing.rs index cc7d0a1570355..ae042cc808126 100644 --- a/src/librustc_query_system/query/plumbing.rs +++ b/src/librustc_query_system/query/plumbing.rs @@ -264,7 +264,7 @@ where /// Completes the query by updating the query cache with the `result`, /// signals the waiter and forgets the JobOwner, so it won't poison the query #[inline(always)] - fn complete(self, tcx: CTX, result: C::Value, dep_node_index: DepNodeIndex) -> C::Stored { + fn complete(self, result: C::Value, dep_node_index: DepNodeIndex) -> C::Stored { // We can move out of `self` here because we `mem::forget` it below let key = unsafe { ptr::read(&self.key) }; let state = self.state; @@ -278,7 +278,7 @@ where QueryResult::Started(job) => job, QueryResult::Poisoned => panic!(), }; - let result = state.cache.complete(tcx, &mut lock.cache, key, result, dep_node_index); + let result = state.cache.complete(&mut lock.cache, key, result, dep_node_index); (job, result) }; @@ -432,7 +432,7 @@ where tcx.store_diagnostics_for_anon_node(dep_node_index, diagnostics); } - return job.complete(tcx, result, dep_node_index); + return job.complete(result, dep_node_index); } let dep_node = query.to_dep_node(tcx, &key); @@ -458,7 +458,7 @@ where }) }); if let Some((result, dep_node_index)) = loaded { - return job.complete(tcx, result, dep_node_index); + return job.complete(result, dep_node_index); } } @@ -609,7 +609,7 @@ where } } - let result = job.complete(tcx, result, dep_node_index); + let result = job.complete(result, dep_node_index); (result, dep_node_index) } From b38e571e9cdfa0e98d10c959117ed8e3e7bd280d Mon Sep 17 00:00:00 2001 From: Ruben Gonzalez Date: Wed, 12 Aug 2020 23:42:10 +0200 Subject: [PATCH 07/13] Fix E0741 error code explanation Use OK ! like other explanations --- src/librustc_error_codes/error_codes/E0477.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0477.md b/src/librustc_error_codes/error_codes/E0477.md index 794456451ef33..9cfefb1de632b 100644 --- a/src/librustc_error_codes/error_codes/E0477.md +++ b/src/librustc_error_codes/error_codes/E0477.md @@ -37,8 +37,7 @@ fn i_want_static_closure(a: F) fn print_string(s: Mutex>) { - i_want_static_closure(move || { // error: this closure has lifetime 'a - // rather than 'static + i_want_static_closure(move || { // ok! println!("{}", s.lock().unwrap().data); }); } From d4593af78fb2929f0c46e5e4fc041092b7195f81 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 12 Aug 2020 15:30:41 -0700 Subject: [PATCH 08/13] Change registered "program name" for -Cllvm-args usage messages While debugging a codegen issue, I tried adding LLVM options with the rustc -Cllvm-args option, and was confused by the error and usage messaging. The LLVM "program name" argument is set to "rustc", and command line error messages make it look like invalid arguments are "rustc" arguments, not LLVM. I changed this argument so error messages and the "-help" usage feedback is easier to understand and react to. (Clang does something similar.) --- src/librustc_codegen_llvm/llvm_util.rs | 3 ++- src/test/ui/unknown-llvm-arg.rs | 22 ++++++++++++++++++++++ src/test/ui/unknown-llvm-arg.stderr | 1 + 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/unknown-llvm-arg.rs create mode 100644 src/test/ui/unknown-llvm-arg.stderr diff --git a/src/librustc_codegen_llvm/llvm_util.rs b/src/librustc_codegen_llvm/llvm_util.rs index b631c10334cd1..f0b50459837e9 100644 --- a/src/librustc_codegen_llvm/llvm_util.rs +++ b/src/librustc_codegen_llvm/llvm_util.rs @@ -73,7 +73,8 @@ unsafe fn configure_llvm(sess: &Session) { llvm_c_strs.push(s); } }; - add("rustc", true); // fake program name + // Set the llvm "program name" to make usage and invalid argument messages more clear. + add("rustc -Cllvm-args=\"...\" with", true); if sess.time_llvm_passes() { add("-time-passes", false); } diff --git a/src/test/ui/unknown-llvm-arg.rs b/src/test/ui/unknown-llvm-arg.rs new file mode 100644 index 0000000000000..289bae24f810e --- /dev/null +++ b/src/test/ui/unknown-llvm-arg.rs @@ -0,0 +1,22 @@ +// compile-flags: -Cllvm-args=-not-a-real-llvm-arg +// normalize-stderr-test "--help" -> "-help" +// normalize-stderr-test "\n(\n|.)*" -> "" + +// I'm seeing "--help" locally, but "-help" in CI, so I'm normalizing it to just "-help". + +// Note that the rustc-supplied "program name", given when invoking LLVM, is used by LLVM to +// generate user-facing error messages and a usage (--help) messages. If the program name is +// `rustc`, the usage message in response to `--llvm-args="--help"` starts with: +// ``` +// USAGE: rustc [options] +// ``` +// followed by the list of options not to `rustc` but to `llvm`. +// +// On the other hand, if the program name is set to `rustc -Cllvm-args="..." with`, the usage +// message is more clear: +// ``` +// USAGE: rustc -Cllvm-args="..." with [options] +// ``` +// This test captures the effect of the current program name setting on LLVM command line +// error messages. +fn main() {} diff --git a/src/test/ui/unknown-llvm-arg.stderr b/src/test/ui/unknown-llvm-arg.stderr new file mode 100644 index 0000000000000..e1d3cfea28fdf --- /dev/null +++ b/src/test/ui/unknown-llvm-arg.stderr @@ -0,0 +1 @@ +rustc -Cllvm-args="..." with: Unknown command line argument '-not-a-real-llvm-arg'. Try: 'rustc -Cllvm-args="..." with -help' \ No newline at end of file From 2100e67126ee917864d2c6e3b7bf075e0ca6d52d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Thu, 13 Aug 2020 11:29:20 +0200 Subject: [PATCH 09/13] make rustc-docs component available to rustup --- src/tools/build-manifest/src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/build-manifest/src/main.rs b/src/tools/build-manifest/src/main.rs index 5dede95a85823..0a71361c992f9 100644 --- a/src/tools/build-manifest/src/main.rs +++ b/src/tools/build-manifest/src/main.rs @@ -447,6 +447,7 @@ impl Builder { let mut package = |name, targets| self.package(name, &mut manifest.pkg, targets); package("rustc", HOSTS); package("rustc-dev", HOSTS); + package("rustc-docs", HOSTS); package("cargo", HOSTS); package("rust-mingw", MINGW); package("rust-std", TARGETS); From 08d951768f6f9b98ca5ffe3385adfb3f1935af22 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 11 Aug 2020 15:19:16 +0200 Subject: [PATCH 10/13] self-profile: Cache more query key strings when doing self-profiling. --- .../ty/query/profiling_support.rs | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/librustc_middle/ty/query/profiling_support.rs b/src/librustc_middle/ty/query/profiling_support.rs index 0683dc0201129..9b1837356e305 100644 --- a/src/librustc_middle/ty/query/profiling_support.rs +++ b/src/librustc_middle/ty/query/profiling_support.rs @@ -1,8 +1,9 @@ use crate::ty::context::TyCtxt; +use crate::ty::WithOptConstParam; use measureme::{StringComponent, StringId}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::profiling::SelfProfiler; -use rustc_hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::definitions::DefPathData; use rustc_query_system::query::QueryCache; use rustc_query_system::query::QueryState; @@ -154,6 +155,49 @@ impl SpecIntoSelfProfilingString for DefIndex { } } +impl SpecIntoSelfProfilingString for LocalDefId { + fn spec_to_self_profile_string( + &self, + builder: &mut QueryKeyStringBuilder<'_, '_, '_>, + ) -> StringId { + builder.def_id_to_string_id(DefId { krate: LOCAL_CRATE, index: self.local_def_index }) + } +} + +impl SpecIntoSelfProfilingString for WithOptConstParam { + fn spec_to_self_profile_string( + &self, + builder: &mut QueryKeyStringBuilder<'_, '_, '_>, + ) -> StringId { + // We print `WithOptConstParam` values as tuples to make them shorter + // and more readable, without losing information: + // + // "WithOptConstParam { did: foo::bar, const_param_did: Some(foo::baz) }" + // becomes "(foo::bar, foo::baz)" and + // "WithOptConstParam { did: foo::bar, const_param_did: None }" + // becomes "(foo::bar, _)". + + let did = StringComponent::Ref(self.did.to_self_profile_string(builder)); + + let const_param_did = if let Some(const_param_did) = self.const_param_did { + let const_param_did = builder.def_id_to_string_id(const_param_did); + StringComponent::Ref(const_param_did) + } else { + StringComponent::Value("_") + }; + + let components = [ + StringComponent::Value("("), + did, + StringComponent::Value(", "), + const_param_did, + StringComponent::Value(")"), + ]; + + builder.profiler.alloc_string(&components[..]) + } +} + impl SpecIntoSelfProfilingString for (T0, T1) where T0: SpecIntoSelfProfilingString, From 2338903260e60e470268d92b7e4894d18a598d59 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 13 Aug 2020 08:09:00 +0200 Subject: [PATCH 11/13] fn type: structure, and talk a bit more about ABIs and how to create them --- library/std/src/primitive_docs.rs | 57 ++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/library/std/src/primitive_docs.rs b/library/std/src/primitive_docs.rs index f9c96b7c3d4b3..565cf62c824ea 100644 --- a/library/std/src/primitive_docs.rs +++ b/library/std/src/primitive_docs.rs @@ -1060,6 +1060,8 @@ mod prim_ref {} /// not be null, so if you want to pass a function pointer over FFI and be able to accommodate null /// pointers, make your type `Option` with your required signature. /// +/// ### Safety +/// /// Plain function pointers are obtained by casting either plain functions, or closures that don't /// capture an environment: /// @@ -1097,23 +1099,60 @@ mod prim_ref {} /// let really_safe_ptr: unsafe fn(usize) -> usize = add_one; /// ``` /// -/// On top of that, function pointers can vary based on what ABI they use. This is achieved by -/// adding the `extern` keyword to the type name, followed by the ABI in question. For example, -/// `fn()` is different from `extern "C" fn()`, which itself is different from `extern "stdcall" -/// fn()`, and so on for the various ABIs that Rust supports. Non-`extern` functions have an ABI -/// of `"Rust"`, and `extern` functions without an explicit ABI have an ABI of `"C"`. For more -/// information, see [the nomicon's section on foreign calling conventions][nomicon-abi]. +/// ### ABI +/// +/// On top of that, function pointers can vary based on what ABI they use. This +/// is achieved by adding the `extern` keyword before the type, followed by the +/// ABI in question. The default ABI is "Rust", i.e., `fn()` is the exact same +/// type as `extern "Rust" fn()`. A pointer to a function with C ABI would have +/// type `extern "C" fn()`. +/// +/// `extern "ABI" { ... }` blocks declare functions with ABI "ABI". The default +/// here is "C", i.e., functions declared in an `extern {...}` block have "C" +/// ABI. +/// +/// For more information and a list of supported ABIs, see [the nomicon's +/// section on foreign calling conventions][nomicon-abi]. /// -/// [nomicon-abi]: ../nomicon/ffi.html#foreign-calling-conventions +/// ### Variadic functions /// /// Extern function declarations with the "C" or "cdecl" ABIs can also be *variadic*, allowing them -/// to be called with a variable number of arguments. Normal rust functions, even those with an +/// to be called with a variable number of arguments. Normal Rust functions, even those with an /// `extern "ABI"`, cannot be variadic. For more information, see [the nomicon's section on /// variadic functions][nomicon-variadic]. /// /// [nomicon-variadic]: ../nomicon/ffi.html#variadic-functions /// -/// These markers can be combined, so `unsafe extern "stdcall" fn()` is a valid type. +/// ### Creating function pointers +/// +/// When `bar` is the name of a function, then the expression `bar` is *not* a +/// function pointer. Rather, it denotes a value of an unnameable type that +/// uniquely identifies the function `bar`. The value is zero-sized because the +/// type already identifies the function. This has the advantage that "calling" +/// the value (it implements the `Fn*` traits) does not require dynamic +/// dispatch. +/// +/// This zero-sized type *coerces* to a regular function pointer. For example: +/// +/// ```rust +/// use std::mem; +/// +/// fn bar(x: i32) {} +/// +/// let not_bar_ptr = bar; // `not_bar_ptr` is zero-sized, uniquely identifying `bar` +/// assert_eq!(mem::size_of_val(¬_bar_ptr), 0); +/// +/// let bar_ptr: fn(i32) = not_bar_ptr; // force coercion to function pointer +/// assert_eq!(mem::size_of_val(&bar_ptr), mem::size_of::()); +/// +/// let footgun = &bar; // this is a shared reference to the zero-sized type identifying `bar` +/// ``` +/// +/// The last line shows that `&bar` is not a function pointer either. Rather, it +/// is a reference to the function-specific ZST. `&bar` is basically never what you +/// want when `bar` is a function. +/// +/// ### Traits /// /// Function pointers implement the following traits: /// From 264434fea56a6cefa08223331692cf162f97f8f9 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 13 Aug 2020 17:04:28 -0300 Subject: [PATCH 12/13] Prioritization WG: Open Zulip topics only for `I-prioritize` issues --- triagebot.toml | 91 -------------------------------------------------- 1 file changed, 91 deletions(-) diff --git a/triagebot.toml b/triagebot.toml index 8fe89095ae99f..707e381b06e47 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -99,94 +99,3 @@ message_on_add = """\ - Needs `I-nominated`? """ message_on_remove = "Issue #{number}'s prioritization request has been removed." - -[notify-zulip."I-nominated"] -required_labels = ["T-compiler"] -zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts -topic = "I-nominated #{number} {title}" -message_on_add = """\ -@*WG-prioritization/alerts* #{number} has been nominated for discussion in `T-compiler` meeting. - -# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-i-nominated-issues) -- Already discussed? -- Worth the meeting time? -- Add agenda entry: - - Why nominated? - - Assignee? - - Issue? PR? What's the status? - - Summary and important details? -""" -message_on_remove = "#{number}'s nomination has been removed." - -[notify-zulip."beta-nominated"] -zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts -topic = "Backport #{number} {title}" -message_on_add = """\ -@*WG-prioritization/alerts* PR #{number} has been requested for beta backport. - -# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-stablebeta-nominations) -Prepare agenda entry: -- Why nominated? -- Author, assignee? -- Important details? -""" -message_on_remove = "PR #{number}'s beta backport request has been removed." - -[notify-zulip."stable-nominated"] -zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts -topic = "Backport #{number} {title}" -message_on_add = """\ -@*WG-prioritization/alerts* PR #{number} has been requested for stable backport. - -# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-stablebeta-nominations) -Prepare agenda entry: -- Why nominated? -- Author, assignee? -- Important details? -""" -message_on_remove = "PR #{number}'s stable backport request has been removed." - -[notify-zulip."S-waiting-on-team"] -required_labels = ["T-compiler"] -zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts -topic = "S-waiting-on-team #{number} {title}" -message_on_add = """\ -@*WG-prioritization/alerts* PR #{number} is waiting on `T-compiler`. - -# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-prs-waiting-on-team) -- Prepare agenda entry: - - What is it waiting for? - - Important details? -- Could be resolved quickly? Tag `I-nominated`. -""" -message_on_remove = "PR #{number}'s is no longer waiting on `T-compiler`." - -[notify-zulip."P-critical"] -zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts -topic = "P-critical #{number} {title}" -message_on_add = """\ -@*WG-prioritization/alerts* issue #{number} has been assigned `P-critical`. - -# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-p-critical-and-unassigned-p-high-regressions) -- Notify people/groups? -- Assign if possible? -- Add to agenda: - - Assignee? - - Summary and important details? -- Other actions to move forward? -""" - -[notify-zulip."P-high"] -required_labels = ["regression-from-stable-to-[bn]*"] # only nightly and beta regressions -zulip_stream = 245100 # #t-compiler/wg-prioritization/alerts -topic = "P-high regression #{number} {title}" -message_on_add = """\ -@*WG-prioritization/alerts* issue #{number} has been assigned `P-high` and is a regression. - -# [Procedure](https://forge.rust-lang.org/compiler/prioritization/procedure.html#summarize-p-critical-and-unassigned-p-high-regressions) -Is issue assigned? If not: -- Try to find an assignee? -- Otherwise add to agenda: - - Mark as unassigned. - - Summary and important details? -""" From 9302c17d18feffb26b4b8812dead594dee1ea858 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Fri, 14 Aug 2020 06:01:15 +0900 Subject: [PATCH 13/13] Disable zlib in LLVM on aarch64-apple-darwin --- src/bootstrap/native.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 48b2cc24d4cd8..3ab50e114c750 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -178,7 +178,7 @@ impl Step for Llvm { .define("LLVM_TARGET_ARCH", target_native.split('-').next().unwrap()) .define("LLVM_DEFAULT_TARGET_TRIPLE", target_native); - if !target.contains("netbsd") { + if !target.contains("netbsd") && target != "aarch64-apple-darwin" { cfg.define("LLVM_ENABLE_ZLIB", "ON"); } else { // FIXME: Enable zlib on NetBSD too