From 1f5568d1a644bd26f7fe806d7c8e40dac15321a2 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Mon, 16 Aug 2021 15:22:36 +0200 Subject: [PATCH 01/23] Emit proper errors when on missing closure braces This commit focuses on emitting clean errors for the following syntax error: ``` Some(42).map(|a| dbg!(a); a ); ``` Previous implementation tried to recover after parsing the closure body (the `dbg` expression) by replacing the next `;` with a `,`, which made the next expression belong to the next function argument. As such, the following errors were emitted (among others): - the semicolon token was not expected, - a is not in scope, - Option::map is supposed to take one argument, not two. This commit allows us to gracefully handle this situation by adding giving the parser the ability to remember when it has just parsed a closure body inside a function call. When this happens, we can treat the unexpected `;` specifically and try to parse as much statements as possible in order to eat the whole block. When we can't parse statements anymore, we generate a clean error indicating that the braces are missing, and return an ExprKind::Err. --- compiler/rustc_parse/src/parser/expr.rs | 15 ++++ compiler/rustc_parse/src/parser/mod.rs | 81 +++++++++++++++++-- .../missing_braces_around_block.fixed | 19 +++++ .../missing_braces_around_block.rs | 19 +++++ .../missing_braces_around_block.stderr | 25 ++++++ 5 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed create mode 100644 src/test/ui/expr/malformed_closure/missing_braces_around_block.rs create mode 100644 src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index a1d3e9adba013..8395a417fdbc3 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -4,6 +4,7 @@ use super::{AttrWrapper, BlockMode, ForceCollect, Parser, PathStyle, Restriction use super::{SemiColonMode, SeqSep, TokenExpectType, TrailingToken}; use crate::maybe_recover_from_interpolated_ty_qpath; +use ast::token::DelimToken; use rustc_ast::ptr::P; use rustc_ast::token::{self, Token, TokenKind}; use rustc_ast::tokenstream::Spacing; @@ -91,6 +92,8 @@ impl<'a> Parser<'a> { /// Parses an expression. #[inline] pub fn parse_expr(&mut self) -> PResult<'a, P> { + self.last_closure_body.take(); + self.parse_expr_res(Restrictions::empty(), None) } @@ -1733,6 +1736,18 @@ impl<'a> Parser<'a> { self.sess.gated_spans.gate(sym::async_closure, span); } + // Disable recovery for closure body + self.last_closure_body = Some(decl_hi); + + if self.token.kind == TokenKind::Semi && self.token_cursor.frame.delim == DelimToken::Paren + { + // It is likely that the closure body is a block but where the + // braces have been removed. We will recover and eat the next + // statements later in the parsing process. + + return Ok(self.mk_expr_err(lo.to(body.span))); + } + Ok(self.mk_expr( lo.to(body.span), ExprKind::Closure(capture_clause, asyncness, movability, decl, body, lo.to(decl_hi)), diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index c34fd02e8066d..b7c9a88b23f43 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -142,6 +142,10 @@ pub struct Parser<'a> { /// If present, this `Parser` is not parsing Rust code but rather a macro call. subparser_name: Option<&'static str>, capture_state: CaptureState, + + /// This allows us to recover when the user forget to add braces around + /// multiple statements in the closure body. + pub last_closure_body: Option, } /// Indicates a range of tokens that should be replaced by @@ -440,6 +444,7 @@ impl<'a> Parser<'a> { replace_ranges: Vec::new(), inner_attr_ranges: Default::default(), }, + last_closure_body: None, }; // Make parser point to the first token. @@ -761,8 +766,11 @@ impl<'a> Parser<'a> { first = false; } else { match self.expect(t) { - Ok(false) => {} + Ok(false) => { + self.last_closure_body.take(); + } Ok(true) => { + self.last_closure_body.take(); recovered = true; break; } @@ -770,11 +778,32 @@ impl<'a> Parser<'a> { let sp = self.prev_token.span.shrink_to_hi(); let token_str = pprust::token_kind_to_string(t); - // Attempt to keep parsing if it was a similar separator. - if let Some(ref tokens) = t.similar_tokens() { - if tokens.contains(&self.token.kind) && !unclosed_delims { - self.bump(); + match self.last_closure_body.take() { + None => { + // Attempt to keep parsing if it was a similar separator. + if let Some(ref tokens) = t.similar_tokens() { + if tokens.contains(&self.token.kind) && !unclosed_delims { + self.bump(); + } + } + } + + Some(right_pipe_span) if self.token.kind == TokenKind::Semi => { + // Finding a semicolon instead of a comma + // after a closure body indicates that the + // closure body may be a block but the user + // forgot to put braces around its + // statements. + + self.recover_missing_braces_around_closure_body( + right_pipe_span, + expect_err, + )?; + + continue; } + + _ => {} } // If this was a missing `@` in a binding pattern @@ -839,6 +868,48 @@ impl<'a> Parser<'a> { Ok((v, trailing, recovered)) } + fn recover_missing_braces_around_closure_body( + &mut self, + right_pipe_span: Span, + mut expect_err: DiagnosticBuilder<'_>, + ) -> PResult<'a, ()> { + let initial_semicolon = self.token.span; + + expect_err.span_help( + initial_semicolon, + "This `;` turns the expression into a statement, which must be placed in a block", + ); + + while self.eat(&TokenKind::Semi) { + let _ = self.parse_stmt(ForceCollect::Yes)?; + } + + expect_err.set_primary_message( + "closure body that contain statements must be surrounded by braces", + ); + + let preceding_pipe_span = right_pipe_span; + let following_token_span = self.token.span; + + expect_err.set_span(vec![preceding_pipe_span, following_token_span]); + + let opening_suggestion_str = " {".to_string(); + let closing_suggestion_str = "}".to_string(); + + expect_err.multipart_suggestion( + "try adding braces", + vec![ + (preceding_pipe_span.shrink_to_hi(), opening_suggestion_str), + (following_token_span.shrink_to_lo(), closing_suggestion_str), + ], + Applicability::MaybeIncorrect, + ); + + expect_err.emit(); + + Ok(()) + } + /// Parses a sequence, not including the closing delimiter. The function /// `f` must consume tokens until reaching the next separator or /// closing bracket. diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed b/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed new file mode 100644 index 0000000000000..651ca6037aea5 --- /dev/null +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed @@ -0,0 +1,19 @@ +// This snippet ensures that no attempt to recover on a semicolon instead of +// comma is made next to a closure body. +// +// If this recovery happens, then plenty of errors are emitted. Here, we expect +// only one error. +// +// This is part of issue #88065: +// https://github.com/rust-lang/rust/issues/88065 + +// run-rustfix + +fn main() { + let num = 5; + (1..num).reduce(|a, b| { + //~^ ERROR: closure body that contain statements must be surrounded by braces + println!("{}", a); + a * b + }).unwrap(); +} diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs b/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs new file mode 100644 index 0000000000000..409ebcd65ad5e --- /dev/null +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs @@ -0,0 +1,19 @@ +// This snippet ensures that no attempt to recover on a semicolon instead of +// comma is made next to a closure body. +// +// If this recovery happens, then plenty of errors are emitted. Here, we expect +// only one error. +// +// This is part of issue #88065: +// https://github.com/rust-lang/rust/issues/88065 + +// run-rustfix + +fn main() { + let num = 5; + (1..num).reduce(|a, b| + //~^ ERROR: closure body that contain statements must be surrounded by braces + println!("{}", a); + a * b + ).unwrap(); +} diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr new file mode 100644 index 0000000000000..260440a74999f --- /dev/null +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr @@ -0,0 +1,25 @@ +error: closure body that contain statements must be surrounded by braces + --> $DIR/missing_braces_around_block.rs:14:26 + | +LL | (1..num).reduce(|a, b| + | ^ +... +LL | ).unwrap(); + | ^ + | +help: This `;` turns the expression into a statement, which must be placed in a block + --> $DIR/missing_braces_around_block.rs:16:26 + | +LL | println!("{}", a); + | ^ +help: try adding braces + | +LL ~ (1..num).reduce(|a, b| { +LL | +LL | println!("{}", a); +LL | a * b +LL ~ }).unwrap(); + | + +error: aborting due to previous error + From 3bdda1cec888cc2e63ae9049610e58be066800c1 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Wed, 1 Sep 2021 09:36:44 +0200 Subject: [PATCH 02/23] Fix grammatical error in error message --- compiler/rustc_parse/src/parser/mod.rs | 2 +- .../ui/expr/malformed_closure/missing_braces_around_block.fixed | 2 +- .../ui/expr/malformed_closure/missing_braces_around_block.rs | 2 +- .../expr/malformed_closure/missing_braces_around_block.stderr | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index b7c9a88b23f43..7ee7dd07a8a0c 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -885,7 +885,7 @@ impl<'a> Parser<'a> { } expect_err.set_primary_message( - "closure body that contain statements must be surrounded by braces", + "closure bodies that contain statements must be surrounded by braces", ); let preceding_pipe_span = right_pipe_span; diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed b/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed index 651ca6037aea5..c50b9a12b6d44 100644 --- a/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed @@ -12,7 +12,7 @@ fn main() { let num = 5; (1..num).reduce(|a, b| { - //~^ ERROR: closure body that contain statements must be surrounded by braces + //~^ ERROR: closure bodies that contain statements must be surrounded by braces println!("{}", a); a * b }).unwrap(); diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs b/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs index 409ebcd65ad5e..58c81f3a6e2a9 100644 --- a/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs @@ -12,7 +12,7 @@ fn main() { let num = 5; (1..num).reduce(|a, b| - //~^ ERROR: closure body that contain statements must be surrounded by braces + //~^ ERROR: closure bodies that contain statements must be surrounded by braces println!("{}", a); a * b ).unwrap(); diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr index 260440a74999f..b35adbdd26ec3 100644 --- a/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr @@ -1,4 +1,4 @@ -error: closure body that contain statements must be surrounded by braces +error: closure bodies that contain statements must be surrounded by braces --> $DIR/missing_braces_around_block.rs:14:26 | LL | (1..num).reduce(|a, b| From eabe41c3502356104342396bd79ef2155530529d Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Wed, 1 Sep 2021 15:31:22 +0200 Subject: [PATCH 03/23] Fix misplaced match arm --- compiler/rustc_parse/src/parser/mod.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 7ee7dd07a8a0c..56e428c6d3f03 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -779,15 +779,6 @@ impl<'a> Parser<'a> { let token_str = pprust::token_kind_to_string(t); match self.last_closure_body.take() { - None => { - // Attempt to keep parsing if it was a similar separator. - if let Some(ref tokens) = t.similar_tokens() { - if tokens.contains(&self.token.kind) && !unclosed_delims { - self.bump(); - } - } - } - Some(right_pipe_span) if self.token.kind == TokenKind::Semi => { // Finding a semicolon instead of a comma // after a closure body indicates that the @@ -803,7 +794,14 @@ impl<'a> Parser<'a> { continue; } - _ => {} + _ => { + // Attempt to keep parsing if it was a similar separator. + if let Some(ref tokens) = t.similar_tokens() { + if tokens.contains(&self.token.kind) && !unclosed_delims { + self.bump(); + } + } + } } // If this was a missing `@` in a binding pattern From e8e76b7f784d3e77c3a20f57db40aff75c4aa49b Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Wed, 1 Sep 2021 15:53:57 +0200 Subject: [PATCH 04/23] Put the expr error inside the closure body --- compiler/rustc_parse/src/parser/expr.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 8395a417fdbc3..fd28d79f1c62b 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1719,7 +1719,7 @@ impl<'a> Parser<'a> { let capture_clause = self.parse_capture_clause()?; let decl = self.parse_fn_block_decl()?; let decl_hi = self.prev_token.span; - let body = match decl.output { + let mut body = match decl.output { FnRetTy::Default(_) => { let restrictions = self.restrictions - Restrictions::STMT_EXPR; self.parse_expr_res(restrictions, None)? @@ -1744,8 +1744,7 @@ impl<'a> Parser<'a> { // It is likely that the closure body is a block but where the // braces have been removed. We will recover and eat the next // statements later in the parsing process. - - return Ok(self.mk_expr_err(lo.to(body.span))); + body = self.mk_expr_err(body.span); } Ok(self.mk_expr( From dbe56f467b36a29f989ea949e648ba5a4365ab88 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Wed, 1 Sep 2021 17:12:37 +0200 Subject: [PATCH 05/23] Add ruby-style closure ui tests --- .../malformed_closure/ruby_style_closure.rs | 8 ++++++++ .../ruby_style_closure.stderr | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 src/test/ui/expr/malformed_closure/ruby_style_closure.rs create mode 100644 src/test/ui/expr/malformed_closure/ruby_style_closure.stderr diff --git a/src/test/ui/expr/malformed_closure/ruby_style_closure.rs b/src/test/ui/expr/malformed_closure/ruby_style_closure.rs new file mode 100644 index 0000000000000..9f35dfc621368 --- /dev/null +++ b/src/test/ui/expr/malformed_closure/ruby_style_closure.rs @@ -0,0 +1,8 @@ +fn main() { + let p = Some(45).and_then({ + //~^ expected a `FnOnce<({integer},)>` closure, found `Option<_>` + |x| println!("doubling {}", x); + Some(x * 2) + //~^ ERROR: cannot find value `x` in this scope + }); +} diff --git a/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr b/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr new file mode 100644 index 0000000000000..88cf132bbb18c --- /dev/null +++ b/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr @@ -0,0 +1,18 @@ +error[E0425]: cannot find value `x` in this scope + --> $DIR/ruby_style_closure.rs:5:14 + | +LL | Some(x * 2) + | ^ not found in this scope + +error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>` + --> $DIR/ruby_style_closure.rs:2:22 + | +LL | let p = Some(45).and_then({ + | ^^^^^^^^ expected an `FnOnce<({integer},)>` closure, found `Option<_>` + | + = help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>` + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0277, E0425. +For more information about an error, try `rustc --explain E0277`. From 733bdd079a3c09aa3baa47ad65ae134f07b9dbcd Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Wed, 1 Sep 2021 20:07:56 -0700 Subject: [PATCH 06/23] fix(rustc): suggest `items` be borrowed in `for i in items[x..]` Fixes #87994 --- .../src/traits/error_reporting/suggestions.rs | 17 +++++++-- src/test/ui/suggestions/slice-issue-87994.rs | 7 ++++ .../ui/suggestions/slice-issue-87994.stderr | 37 +++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/suggestions/slice-issue-87994.rs create mode 100644 src/test/ui/suggestions/slice-issue-87994.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index db3432b01422f..608add1d91c9f 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -23,7 +23,7 @@ use rustc_middle::ty::{ use rustc_middle::ty::{TypeAndMut, TypeckResults}; use rustc_span::def_id::LOCAL_CRATE; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP}; +use rustc_span::{BytePos, DesugaringKind, ExpnKind, ForLoopLoc, MultiSpan, Span, DUMMY_SP}; use rustc_target::spec::abi; use std::fmt; @@ -680,7 +680,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { points_at_arg: bool, has_custom_message: bool, ) -> bool { - if !points_at_arg { + let span = obligation.cause.span; + let points_at_for_iter = matches!( + span.ctxt().outer_expn_data().kind, + ExpnKind::Desugaring(DesugaringKind::ForLoop(ForLoopLoc::IntoIter)) + ); + + if !points_at_arg && !points_at_for_iter { return false; } @@ -695,7 +701,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { never_suggest_borrow.push(self.tcx.get_diagnostic_item(sym::send_trait).unwrap()); - let span = obligation.cause.span; let param_env = obligation.param_env; let trait_ref = trait_ref.skip_binder(); @@ -754,7 +759,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); // This if is to prevent a special edge-case - if !span.from_expansion() { + if matches!( + span.ctxt().outer_expn_data().kind, + ExpnKind::Root + | ExpnKind::Desugaring(DesugaringKind::ForLoop(ForLoopLoc::IntoIter)) + ) { // We don't want a borrowing suggestion on the fields in structs, // ``` // struct Foo { diff --git a/src/test/ui/suggestions/slice-issue-87994.rs b/src/test/ui/suggestions/slice-issue-87994.rs new file mode 100644 index 0000000000000..98a936ab2fdd5 --- /dev/null +++ b/src/test/ui/suggestions/slice-issue-87994.rs @@ -0,0 +1,7 @@ +fn main() { + let v = vec![1i32, 2, 3]; + for _ in v[1..] { + //~^ ERROR [i32]` is not an iterator [E0277] + //~^^ ERROR known at compilation time + } +} diff --git a/src/test/ui/suggestions/slice-issue-87994.stderr b/src/test/ui/suggestions/slice-issue-87994.stderr new file mode 100644 index 0000000000000..0c69bec22109b --- /dev/null +++ b/src/test/ui/suggestions/slice-issue-87994.stderr @@ -0,0 +1,37 @@ +error[E0277]: the size for values of type `[i32]` cannot be known at compilation time + --> $DIR/slice-issue-87994.rs:3:12 + | +LL | for _ in v[1..] { + | ^^^^^^ + | | + | expected an implementor of trait `IntoIterator` + | help: consider borrowing here: `&v[1..]` + | + = note: the trait bound `[i32]: IntoIterator` is not satisfied + = note: required because of the requirements on the impl of `IntoIterator` for `[i32]` +note: required by `into_iter` + --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL + | +LL | fn into_iter(self) -> Self::IntoIter; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0277]: `[i32]` is not an iterator + --> $DIR/slice-issue-87994.rs:3:12 + | +LL | for _ in v[1..] { + | ^^^^^^ + | | + | expected an implementor of trait `IntoIterator` + | help: consider borrowing here: `&v[1..]` + | + = note: the trait bound `[i32]: IntoIterator` is not satisfied + = note: required because of the requirements on the impl of `IntoIterator` for `[i32]` +note: required by `into_iter` + --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL + | +LL | fn into_iter(self) -> Self::IntoIter; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. From f23003dff12fdb6c3ba09a55b1323cd2692e50d1 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Wed, 1 Sep 2021 20:39:11 -0700 Subject: [PATCH 07/23] fix(test): update with `&mut` suggestion --- src/test/ui/issues/issue-20605.stderr | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/test/ui/issues/issue-20605.stderr b/src/test/ui/issues/issue-20605.stderr index 25a90df45613b..ce3ab6b599864 100644 --- a/src/test/ui/issues/issue-20605.stderr +++ b/src/test/ui/issues/issue-20605.stderr @@ -2,9 +2,12 @@ error[E0277]: the size for values of type `dyn Iterator` cann --> $DIR/issue-20605.rs:2:17 | LL | for item in *things { *item = 0 } - | ^^^^^^^ doesn't have a size known at compile-time + | ^^^^^^^ + | | + | expected an implementor of trait `IntoIterator` + | help: consider mutably borrowing here: `&mut *things` | - = help: the trait `Sized` is not implemented for `dyn Iterator` + = note: the trait bound `dyn Iterator: IntoIterator` is not satisfied = note: required because of the requirements on the impl of `IntoIterator` for `dyn Iterator` note: required by `into_iter` --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL From 3a334a0e159141d948a26ae15a6288287ae06a7b Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Thu, 2 Sep 2021 20:13:40 +0200 Subject: [PATCH 08/23] Add comment about ruby_style_closure status --- src/test/ui/expr/malformed_closure/ruby_style_closure.rs | 8 ++++++++ .../ui/expr/malformed_closure/ruby_style_closure.stderr | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/test/ui/expr/malformed_closure/ruby_style_closure.rs b/src/test/ui/expr/malformed_closure/ruby_style_closure.rs index 9f35dfc621368..e4341e196877b 100644 --- a/src/test/ui/expr/malformed_closure/ruby_style_closure.rs +++ b/src/test/ui/expr/malformed_closure/ruby_style_closure.rs @@ -1,3 +1,11 @@ +// Part of issue #27300. +// The problem here is that ruby-style closures are parsed as blocks whose +// first statement is a closure. See the issue for more details: +// https://github.com/rust-lang/rust/issues/27300 + +// Note: this test represents what the compiler currently emits. The error +// message will be improved later. + fn main() { let p = Some(45).and_then({ //~^ expected a `FnOnce<({integer},)>` closure, found `Option<_>` diff --git a/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr b/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr index 88cf132bbb18c..99df0632b4c33 100644 --- a/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr +++ b/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr @@ -1,11 +1,11 @@ error[E0425]: cannot find value `x` in this scope - --> $DIR/ruby_style_closure.rs:5:14 + --> $DIR/ruby_style_closure.rs:13:14 | LL | Some(x * 2) | ^ not found in this scope error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>` - --> $DIR/ruby_style_closure.rs:2:22 + --> $DIR/ruby_style_closure.rs:10:22 | LL | let p = Some(45).and_then({ | ^^^^^^^^ expected an `FnOnce<({integer},)>` closure, found `Option<_>` From 191bb8385b54dbfd7a5252993d5575dad91a1c80 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Fri, 3 Sep 2021 09:01:05 +0200 Subject: [PATCH 09/23] Put some notes in a separate note block --- compiler/rustc_parse/src/parser/expr.rs | 14 ++++--- compiler/rustc_parse/src/parser/mod.rs | 39 +++++++++++++++---- .../missing_braces_around_block.stderr | 17 +++++++- 3 files changed, 56 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index fd28d79f1c62b..9588d20fdf586 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1736,9 +1736,6 @@ impl<'a> Parser<'a> { self.sess.gated_spans.gate(sym::async_closure, span); } - // Disable recovery for closure body - self.last_closure_body = Some(decl_hi); - if self.token.kind == TokenKind::Semi && self.token_cursor.frame.delim == DelimToken::Paren { // It is likely that the closure body is a block but where the @@ -1747,11 +1744,18 @@ impl<'a> Parser<'a> { body = self.mk_expr_err(body.span); } - Ok(self.mk_expr( + let body_span = body.span; + + let clo = self.mk_expr( lo.to(body.span), ExprKind::Closure(capture_clause, asyncness, movability, decl, body, lo.to(decl_hi)), attrs, - )) + ); + + // Disable recovery for closure body + self.last_closure_body = Some((clo.span, decl_hi, body_span)); + + Ok(clo) } /// Parses an optional `move` prefix to a closure-like construct. diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 56e428c6d3f03..8b16100eaea53 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -35,6 +35,7 @@ use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, FatalError use rustc_session::parse::ParseSess; use rustc_span::source_map::{Span, DUMMY_SP}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; +use rustc_span::MultiSpan; use tracing::debug; use std::ops::Range; @@ -145,7 +146,11 @@ pub struct Parser<'a> { /// This allows us to recover when the user forget to add braces around /// multiple statements in the closure body. - pub last_closure_body: Option, + pub last_closure_body: Option<( + Span, /* The whole body. */ + Span, /* The closing `|` of the closure declarator. */ + Span, /* What we parsed as closure body. */ + )>, } /// Indicates a range of tokens that should be replaced by @@ -779,7 +784,9 @@ impl<'a> Parser<'a> { let token_str = pprust::token_kind_to_string(t); match self.last_closure_body.take() { - Some(right_pipe_span) if self.token.kind == TokenKind::Semi => { + Some((closure_span, right_pipe_span, expr_span)) + if self.token.kind == TokenKind::Semi => + { // Finding a semicolon instead of a comma // after a closure body indicates that the // closure body may be a block but the user @@ -787,7 +794,9 @@ impl<'a> Parser<'a> { // statements. self.recover_missing_braces_around_closure_body( + closure_span, right_pipe_span, + expr_span, expect_err, )?; @@ -868,16 +877,13 @@ impl<'a> Parser<'a> { fn recover_missing_braces_around_closure_body( &mut self, + closure_span: Span, right_pipe_span: Span, + expr_span: Span, mut expect_err: DiagnosticBuilder<'_>, ) -> PResult<'a, ()> { let initial_semicolon = self.token.span; - expect_err.span_help( - initial_semicolon, - "This `;` turns the expression into a statement, which must be placed in a block", - ); - while self.eat(&TokenKind::Semi) { let _ = self.parse_stmt(ForceCollect::Yes)?; } @@ -889,6 +895,25 @@ impl<'a> Parser<'a> { let preceding_pipe_span = right_pipe_span; let following_token_span = self.token.span; + let mut first_note = MultiSpan::from(vec![initial_semicolon]); + first_note.push_span_label( + initial_semicolon, + "this `;` turns the preceding expression into a statement".to_string(), + ); + first_note.push_span_label( + expr_span, + "this expression is a statement because of the trailing semicolon".to_string(), + ); + expect_err.span_note(first_note, "statement found outside of a block"); + + let mut second_note = MultiSpan::from(vec![closure_span]); + second_note.push_span_label(closure_span, "this is the parsed closure...".to_string()); + second_note.push_span_label( + following_token_span, + "...but likely you meant the closure to end here".to_string(), + ); + expect_err.span_note(second_note, "the closure body may be incorrectly delimited"); + expect_err.set_span(vec![preceding_pipe_span, following_token_span]); let opening_suggestion_str = " {".to_string(); diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr index b35adbdd26ec3..1ccd7551c9384 100644 --- a/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr @@ -7,11 +7,24 @@ LL | (1..num).reduce(|a, b| LL | ).unwrap(); | ^ | -help: This `;` turns the expression into a statement, which must be placed in a block +note: statement found outside of a block --> $DIR/missing_braces_around_block.rs:16:26 | LL | println!("{}", a); - | ^ + | -----------------^ this `;` turns the preceding expression into a statement + | | + | this expression is a statement because of the trailing semicolon +note: the closure body may be incorrectly delimited + --> $DIR/missing_braces_around_block.rs:14:21 + | +LL | (1..num).reduce(|a, b| + | _____________________^ +LL | | +LL | | println!("{}", a); + | |_________________________^ this is the parsed closure... +LL | a * b +LL | ).unwrap(); + | - ...but likely you meant the closure to end here help: try adding braces | LL ~ (1..num).reduce(|a, b| { From 41ac2c669b9b87617cf61b93a13d4e8c1c4e5dfc Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Fri, 3 Sep 2021 11:55:01 +0200 Subject: [PATCH 10/23] Fix merge imports --- compiler/rustc_parse/src/parser/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 703fa40b49ac0..2c314e8f45c3a 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -35,7 +35,6 @@ use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, FatalError use rustc_session::parse::ParseSess; use rustc_span::source_map::{MultiSpan, Span, DUMMY_SP}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::MultiSpan; use tracing::debug; use std::ops::Range; From 4107396613e117c8a979fd78a7d5edde2bba9dfe Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Fri, 3 Sep 2021 20:51:50 +0200 Subject: [PATCH 11/23] Group closure-related spans in a structure --- compiler/rustc_parse/src/parser/expr.rs | 8 ++++-- compiler/rustc_parse/src/parser/mod.rs | 38 ++++++++++++------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 9588d20fdf586..dc747904689fe 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1,6 +1,8 @@ use super::pat::{RecoverColon, RecoverComma, PARAM_EXPECTED}; use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign}; -use super::{AttrWrapper, BlockMode, ForceCollect, Parser, PathStyle, Restrictions, TokenType}; +use super::{ + AttrWrapper, BlockMode, ClosureSpans, ForceCollect, Parser, PathStyle, Restrictions, TokenType, +}; use super::{SemiColonMode, SeqSep, TokenExpectType, TrailingToken}; use crate::maybe_recover_from_interpolated_ty_qpath; @@ -1753,7 +1755,9 @@ impl<'a> Parser<'a> { ); // Disable recovery for closure body - self.last_closure_body = Some((clo.span, decl_hi, body_span)); + let spans = + ClosureSpans { whole_closure: clo.span, closing_pipe: decl_hi, body: body_span }; + self.last_closure_body = Some(spans); Ok(clo) } diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 2c314e8f45c3a..a4980306df8f7 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -142,14 +142,17 @@ pub struct Parser<'a> { /// If present, this `Parser` is not parsing Rust code but rather a macro call. subparser_name: Option<&'static str>, capture_state: CaptureState, - /// This allows us to recover when the user forget to add braces around /// multiple statements in the closure body. - pub last_closure_body: Option<( - Span, /* The whole body. */ - Span, /* The closing `|` of the closure declarator. */ - Span, /* What we parsed as closure body. */ - )>, + pub last_closure_body: Option, +} + +/// Stores span informations about a closure. +#[derive(Clone)] +pub struct ClosureSpans { + pub whole_closure: Span, + pub closing_pipe: Span, + pub body: Span, } /// Indicates a range of tokens that should be replaced by @@ -783,9 +786,7 @@ impl<'a> Parser<'a> { let token_str = pprust::token_kind_to_string(t); match self.last_closure_body.take() { - Some((closure_span, right_pipe_span, expr_span)) - if self.token.kind == TokenKind::Semi => - { + Some(closure_spans) if self.token.kind == TokenKind::Semi => { // Finding a semicolon instead of a comma // after a closure body indicates that the // closure body may be a block but the user @@ -793,9 +794,7 @@ impl<'a> Parser<'a> { // statements. self.recover_missing_braces_around_closure_body( - closure_span, - right_pipe_span, - expr_span, + closure_spans, expect_err, )?; @@ -876,9 +875,7 @@ impl<'a> Parser<'a> { fn recover_missing_braces_around_closure_body( &mut self, - closure_span: Span, - right_pipe_span: Span, - expr_span: Span, + closure_spans: ClosureSpans, mut expect_err: DiagnosticBuilder<'_>, ) -> PResult<'a, ()> { let initial_semicolon = self.token.span; @@ -891,7 +888,7 @@ impl<'a> Parser<'a> { "closure bodies that contain statements must be surrounded by braces", ); - let preceding_pipe_span = right_pipe_span; + let preceding_pipe_span = closure_spans.closing_pipe; let following_token_span = self.token.span; let mut first_note = MultiSpan::from(vec![initial_semicolon]); @@ -900,13 +897,16 @@ impl<'a> Parser<'a> { "this `;` turns the preceding expression into a statement".to_string(), ); first_note.push_span_label( - expr_span, + closure_spans.body, "this expression is a statement because of the trailing semicolon".to_string(), ); expect_err.span_note(first_note, "statement found outside of a block"); - let mut second_note = MultiSpan::from(vec![closure_span]); - second_note.push_span_label(closure_span, "this is the parsed closure...".to_string()); + let mut second_note = MultiSpan::from(vec![closure_spans.whole_closure]); + second_note.push_span_label( + closure_spans.whole_closure, + "this is the parsed closure...".to_string(), + ); second_note.push_span_label( following_token_span, "...but likely you meant the closure to end here".to_string(), From 208a5fd322c6271c1d98b7a80d0d3f9b634494ae Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 3 Sep 2021 17:10:52 -0700 Subject: [PATCH 12/23] Use `summary_opts()` for Markdown summaries It was accidentally changed to use `opts()` in #86451. I also renamed `opts()` to `main_body_opts()` to make this kind of accidental change less likely. --- src/librustdoc/html/markdown.rs | 17 +++++++++-------- src/librustdoc/passes/bare_urls.rs | 4 ++-- src/librustdoc/passes/html_tags.rs | 4 ++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index aa3723eddfcce..ae2a5ac440357 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -48,7 +48,7 @@ use pulldown_cmark::{ mod tests; /// Options for rendering Markdown in the main body of documentation. -pub(crate) fn opts() -> Options { +pub(crate) fn main_body_opts() -> Options { Options::ENABLE_TABLES | Options::ENABLE_FOOTNOTES | Options::ENABLE_STRIKETHROUGH @@ -56,7 +56,7 @@ pub(crate) fn opts() -> Options { | Options::ENABLE_SMART_PUNCTUATION } -/// A subset of [`opts()`] used for rendering summaries. +/// A subset of [`main_body_opts()`] used for rendering summaries. pub(crate) fn summary_opts() -> Options { Options::ENABLE_STRIKETHROUGH | Options::ENABLE_SMART_PUNCTUATION | Options::ENABLE_TABLES } @@ -975,7 +975,7 @@ impl Markdown<'_> { } }; - let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut replacer)); + let p = Parser::new_with_broken_link_callback(md, main_body_opts(), Some(&mut replacer)); let p = p.into_offset_iter(); let mut s = String::with_capacity(md.len() * 3 / 2); @@ -994,7 +994,7 @@ impl MarkdownWithToc<'_> { crate fn into_string(self) -> String { let MarkdownWithToc(md, mut ids, codes, edition, playground) = self; - let p = Parser::new_ext(md, opts()).into_offset_iter(); + let p = Parser::new_ext(md, main_body_opts()).into_offset_iter(); let mut s = String::with_capacity(md.len() * 3 / 2); @@ -1019,7 +1019,7 @@ impl MarkdownHtml<'_> { if md.is_empty() { return String::new(); } - let p = Parser::new_ext(md, opts()).into_offset_iter(); + let p = Parser::new_ext(md, main_body_opts()).into_offset_iter(); // Treat inline HTML as plain text. let p = p.map(|event| match event.0 { @@ -1093,7 +1093,7 @@ fn markdown_summary_with_limit( } }; - let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut replacer)); + let p = Parser::new_with_broken_link_callback(md, summary_opts(), Some(&mut replacer)); let mut p = LinkReplacer::new(p, link_names); let mut buf = HtmlWithLimit::new(length_limit); @@ -1240,7 +1240,8 @@ crate fn markdown_links(md: &str) -> Vec { }); None }; - let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut push)).into_offset_iter(); + let p = Parser::new_with_broken_link_callback(md, main_body_opts(), Some(&mut push)) + .into_offset_iter(); // There's no need to thread an IdMap through to here because // the IDs generated aren't going to be emitted anywhere. @@ -1279,7 +1280,7 @@ crate fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec DocFolder for BareUrlsLinter<'a, 'tcx> { }); }; - let mut p = Parser::new_ext(&dox, opts()).into_offset_iter(); + let mut p = Parser::new_ext(&dox, main_body_opts()).into_offset_iter(); while let Some((event, range)) = p.next() { match event { diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index f29d38e3e078a..a0144a5298eba 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -2,7 +2,7 @@ use super::Pass; use crate::clean::*; use crate::core::DocContext; use crate::fold::DocFolder; -use crate::html::markdown::opts; +use crate::html::markdown::main_body_opts; use core::ops::Range; use pulldown_cmark::{Event, Parser, Tag}; use std::iter::Peekable; @@ -192,7 +192,7 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { let mut is_in_comment = None; let mut in_code_block = false; - let p = Parser::new_ext(&dox, opts()).into_offset_iter(); + let p = Parser::new_ext(&dox, main_body_opts()).into_offset_iter(); for (event, range) in p { match event { From 2cc7b7c5f243713c72114168890ba46dd0c6e4ce Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 3 Sep 2021 17:17:24 -0700 Subject: [PATCH 13/23] Enable all main body Markdown options for summaries This fixes odd renderings when these features are used in the first paragraph of documentation for an item. This is an extension of #87270. --- src/librustdoc/html/markdown.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index ae2a5ac440357..a5938149c1f9d 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -56,9 +56,13 @@ pub(crate) fn main_body_opts() -> Options { | Options::ENABLE_SMART_PUNCTUATION } -/// A subset of [`main_body_opts()`] used for rendering summaries. +/// Options for rendering Markdown in summaries (e.g., in search results). pub(crate) fn summary_opts() -> Options { - Options::ENABLE_STRIKETHROUGH | Options::ENABLE_SMART_PUNCTUATION | Options::ENABLE_TABLES + Options::ENABLE_TABLES + | Options::ENABLE_FOOTNOTES + | Options::ENABLE_STRIKETHROUGH + | Options::ENABLE_TASKLISTS + | Options::ENABLE_SMART_PUNCTUATION } /// When `to_string` is called, this struct will emit the HTML corresponding to From e9e8b61e4875e447a4e6760b7fc590d411da3793 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Sat, 4 Sep 2021 11:52:29 +0200 Subject: [PATCH 14/23] Rename clo -> closure --- compiler/rustc_parse/src/parser/expr.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index dc747904689fe..3731425869d95 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1748,7 +1748,7 @@ impl<'a> Parser<'a> { let body_span = body.span; - let clo = self.mk_expr( + let closure = self.mk_expr( lo.to(body.span), ExprKind::Closure(capture_clause, asyncness, movability, decl, body, lo.to(decl_hi)), attrs, @@ -1756,10 +1756,10 @@ impl<'a> Parser<'a> { // Disable recovery for closure body let spans = - ClosureSpans { whole_closure: clo.span, closing_pipe: decl_hi, body: body_span }; + ClosureSpans { whole_closure: closure.span, closing_pipe: decl_hi, body: body_span }; self.last_closure_body = Some(spans); - Ok(clo) + Ok(closure) } /// Parses an optional `move` prefix to a closure-like construct. From f91161d2950762cdb5b09f3ea406f5d6bf9179e3 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Sat, 4 Sep 2021 11:55:06 +0200 Subject: [PATCH 15/23] last_closure_body -> current_closure --- compiler/rustc_parse/src/parser/expr.rs | 4 ++-- compiler/rustc_parse/src/parser/mod.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 3731425869d95..4eacb2ad2e0be 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -94,7 +94,7 @@ impl<'a> Parser<'a> { /// Parses an expression. #[inline] pub fn parse_expr(&mut self) -> PResult<'a, P> { - self.last_closure_body.take(); + self.current_closure.take(); self.parse_expr_res(Restrictions::empty(), None) } @@ -1757,7 +1757,7 @@ impl<'a> Parser<'a> { // Disable recovery for closure body let spans = ClosureSpans { whole_closure: closure.span, closing_pipe: decl_hi, body: body_span }; - self.last_closure_body = Some(spans); + self.current_closure = Some(spans); Ok(closure) } diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index a4980306df8f7..69ff8ceb046b4 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -144,7 +144,7 @@ pub struct Parser<'a> { capture_state: CaptureState, /// This allows us to recover when the user forget to add braces around /// multiple statements in the closure body. - pub last_closure_body: Option, + pub current_closure: Option, } /// Stores span informations about a closure. @@ -451,7 +451,7 @@ impl<'a> Parser<'a> { replace_ranges: Vec::new(), inner_attr_ranges: Default::default(), }, - last_closure_body: None, + current_closure: None, }; // Make parser point to the first token. @@ -774,10 +774,10 @@ impl<'a> Parser<'a> { } else { match self.expect(t) { Ok(false) => { - self.last_closure_body.take(); + self.current_closure.take(); } Ok(true) => { - self.last_closure_body.take(); + self.current_closure.take(); recovered = true; break; } @@ -785,7 +785,7 @@ impl<'a> Parser<'a> { let sp = self.prev_token.span.shrink_to_hi(); let token_str = pprust::token_kind_to_string(t); - match self.last_closure_body.take() { + match self.current_closure.take() { Some(closure_spans) if self.token.kind == TokenKind::Semi => { // Finding a semicolon instead of a comma // after a closure body indicates that the From 99c46c0d20e4f46a5d4207c4d35cca5832d2b713 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Mon, 6 Sep 2021 11:20:34 +0200 Subject: [PATCH 16/23] Slightly reword the error message --- compiler/rustc_parse/src/parser/mod.rs | 2 +- .../expr/malformed_closure/missing_braces_around_block.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 69ff8ceb046b4..5c701fefd17de 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -894,7 +894,7 @@ impl<'a> Parser<'a> { let mut first_note = MultiSpan::from(vec![initial_semicolon]); first_note.push_span_label( initial_semicolon, - "this `;` turns the preceding expression into a statement".to_string(), + "this `;` turns the preceding closure into a statement".to_string(), ); first_note.push_span_label( closure_spans.body, diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr index 1ccd7551c9384..dac9a8cfc69d4 100644 --- a/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr @@ -11,7 +11,7 @@ note: statement found outside of a block --> $DIR/missing_braces_around_block.rs:16:26 | LL | println!("{}", a); - | -----------------^ this `;` turns the preceding expression into a statement + | -----------------^ this `;` turns the preceding closure into a statement | | | this expression is a statement because of the trailing semicolon note: the closure body may be incorrectly delimited From 4a915ac8d9f33567b77b23e90557f92860aa6db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Sat, 4 Sep 2021 13:26:07 +0200 Subject: [PATCH 17/23] fix ICE on hidden tuple variant fields this also renders them as `_`, which rustdoc previously did not. --- src/librustdoc/html/render/print_item.rs | 51 +++++++++++++----------- src/test/rustdoc/issue-88600.rs | 34 ++++++++++++++++ 2 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 src/test/rustdoc/issue-88600.rs diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 39ef641a3ace2..023e47719778b 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -942,15 +942,15 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni } fn print_tuple_struct_fields(w: &mut Buffer, cx: &Context<'_>, s: &[clean::Item]) { - for (i, ty) in s - .iter() - .map(|f| if let clean::StructFieldItem(ref ty) = *f.kind { ty } else { unreachable!() }) - .enumerate() - { + for (i, ty) in s.iter().enumerate() { if i > 0 { w.write_str(", "); } - write!(w, "{}", ty.print(cx)); + match *ty.kind { + clean::StrippedItem(box clean::StructFieldItem(_)) => w.write_str("_"), + clean::StructFieldItem(ref ty) => write!(w, "{}", ty.print(cx)), + _ => unreachable!(), + } } } @@ -1066,24 +1066,27 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum name = variant.name.as_ref().unwrap(), ); for field in fields { - use crate::clean::StructFieldItem; - if let StructFieldItem(ref ty) = *field.kind { - let id = cx.derive_id(format!( - "variant.{}.field.{}", - variant.name.as_ref().unwrap(), - field.name.as_ref().unwrap() - )); - write!( - w, - "\ - \ - {f}: {t}\ - ", - id = id, - f = field.name.as_ref().unwrap(), - t = ty.print(cx) - ); - document(w, cx, field, Some(variant)); + match *field.kind { + clean::StrippedItem(box clean::StructFieldItem(_)) => {} + clean::StructFieldItem(ref ty) => { + let id = cx.derive_id(format!( + "variant.{}.field.{}", + variant.name.as_ref().unwrap(), + field.name.as_ref().unwrap() + )); + write!( + w, + "\ + \ + {f}: {t}\ + ", + id = id, + f = field.name.as_ref().unwrap(), + t = ty.print(cx) + ); + document(w, cx, field, Some(variant)); + } + _ => unreachable!(), } } w.write_str(""); diff --git a/src/test/rustdoc/issue-88600.rs b/src/test/rustdoc/issue-88600.rs new file mode 100644 index 0000000000000..3761805b48b71 --- /dev/null +++ b/src/test/rustdoc/issue-88600.rs @@ -0,0 +1,34 @@ +// This test ensure that #[doc(hidden)] is applied correctly in enum variant fields. + +// Denotes a field which should be hidden. +pub struct H; + +// Denotes a field which should not be hidden (shown). +pub struct S; + +// @has issue_88600/enum.FooEnum.html +pub enum FooEnum { + // @has - '//*[@id="variant.HiddenTupleItem"]//code' 'HiddenTupleItem(_)' + // @count - '//*[@id="variant.HiddenTupleItem.field.0"]' 0 + HiddenTupleItem(#[doc(hidden)] H), + // @has - '//*[@id="variant.MultipleHidden"]//code' 'MultipleHidden(_, _)' + // @count - '//*[@id="variant.MultipleHidden.field.0"]' 0 + // @count - '//*[@id="variant.MultipleHidden.field.1"]' 0 + MultipleHidden(#[doc(hidden)] H, #[doc(hidden)] H), + // @has - '//*[@id="variant.MixedHiddenFirst"]//code' 'MixedHiddenFirst(_, S)' + // @count - '//*[@id="variant.MixedHiddenFirst.field.0"]' 0 + // @has - '//*[@id="variant.MixedHiddenFirst.field.1"]' '1: S' + MixedHiddenFirst(#[doc(hidden)] H, S), + // @has - '//*[@id="variant.MixedHiddenLast"]//code' 'MixedHiddenLast(S, _)' + // @has - '//*[@id="variant.MixedHiddenLast.field.0"]' '0: S' + // @count - '//*[@id="variant.MixedHiddenLast.field.1"]' 0 + MixedHiddenLast(S, #[doc(hidden)] H), + // @has - '//*[@id="variant.HiddenStruct"]//code' 'HiddenStruct' + // @count - '//*[@id="variant.HiddenStruct.field.h"]' 0 + // @has - '//*[@id="variant.HiddenStruct.field.s"]' 's: S' + HiddenStruct { + #[doc(hidden)] + h: H, + s: S, + }, +} From d6ff916c2fe6a47f52fc14e4f6d85e92d07e5e76 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 7 Sep 2021 00:49:43 -0700 Subject: [PATCH 18/23] test: add case for mutating iterator Note that this incorrectly suggests a shared borrow, but at least we know it's happening. --- src/test/ui/suggestions/slice-issue-87994.rs | 9 +++++ .../ui/suggestions/slice-issue-87994.stderr | 36 ++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/test/ui/suggestions/slice-issue-87994.rs b/src/test/ui/suggestions/slice-issue-87994.rs index 98a936ab2fdd5..ecb7f54ea250a 100644 --- a/src/test/ui/suggestions/slice-issue-87994.rs +++ b/src/test/ui/suggestions/slice-issue-87994.rs @@ -4,4 +4,13 @@ fn main() { //~^ ERROR [i32]` is not an iterator [E0277] //~^^ ERROR known at compilation time } + struct K { + n: i32, + } + let mut v2 = vec![K { n: 1 }, K { n: 1 }, K { n: 1 }]; + for i2 in v2[1..] { + //~^ ERROR [K]` is not an iterator [E0277] + //~^^ ERROR known at compilation time + i2.n = 2; + } } diff --git a/src/test/ui/suggestions/slice-issue-87994.stderr b/src/test/ui/suggestions/slice-issue-87994.stderr index 0c69bec22109b..018f62e783daf 100644 --- a/src/test/ui/suggestions/slice-issue-87994.stderr +++ b/src/test/ui/suggestions/slice-issue-87994.stderr @@ -32,6 +32,40 @@ note: required by `into_iter` LL | fn into_iter(self) -> Self::IntoIter; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 2 previous errors +error[E0277]: the size for values of type `[K]` cannot be known at compilation time + --> $DIR/slice-issue-87994.rs:11:13 + | +LL | for i2 in v2[1..] { + | ^^^^^^^ + | | + | expected an implementor of trait `IntoIterator` + | help: consider borrowing here: `&v2[1..]` + | + = note: the trait bound `[K]: IntoIterator` is not satisfied + = note: required because of the requirements on the impl of `IntoIterator` for `[K]` +note: required by `into_iter` + --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL + | +LL | fn into_iter(self) -> Self::IntoIter; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0277]: `[K]` is not an iterator + --> $DIR/slice-issue-87994.rs:11:13 + | +LL | for i2 in v2[1..] { + | ^^^^^^^ + | | + | expected an implementor of trait `IntoIterator` + | help: consider borrowing here: `&v2[1..]` + | + = note: the trait bound `[K]: IntoIterator` is not satisfied + = note: required because of the requirements on the impl of `IntoIterator` for `[K]` +note: required by `into_iter` + --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL + | +LL | fn into_iter(self) -> Self::IntoIter; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0277`. From 532bb80f7fd22bbc9896d9233f2a9ed79071716c Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Tue, 7 Sep 2021 14:49:03 -0400 Subject: [PATCH 19/23] RustWrapper: avoid deleted unclear attribute methods These were deleted in https://reviews.llvm.org/D108614, and in C++ I definitely see the argument for their removal. I didn't try and propagate the changes up into higher layers of rustc in this change because my initial goal was to get rustc working against LLVM HEAD promptly, but I'm happy to follow up with some refactoring to make the API on the Rust side match the LLVM API more directly (though the way the enum works in Rust makes the API less scary IMO). r? @nagisa cc @nikic --- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 88 +++++++++++++++---- 1 file changed, 70 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 4f07a0c67c13f..b3143f14d29cc 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -203,20 +203,59 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) { report_fatal_error("bad AttributeKind"); } +template static inline void AddAttribute(T *t, unsigned Index, Attribute Attr) { +#if LLVM_VERSION_LT(14, 0) + t->addAttribute(Index, Attr); +#else + // TODO(durin42): we should probably surface the explicit functions to Rust + // instead of this switch statement? + switch (Index) { + case AttributeList::ReturnIndex: + t->addRetAttr(Attr); + break; + case AttributeList::FunctionIndex: + t->addFnAttr(Attr); + break; + default: + t->addParamAttr(Index-AttributeList::FirstArgIndex, Attr); + } +#endif +} + extern "C" void LLVMRustAddCallSiteAttribute(LLVMValueRef Instr, unsigned Index, LLVMRustAttribute RustAttr) { CallBase *Call = unwrap(Instr); Attribute Attr = Attribute::get(Call->getContext(), fromRust(RustAttr)); - Call->addAttribute(Index, Attr); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddCallSiteAttrString(LLVMValueRef Instr, unsigned Index, const char *Name) { CallBase *Call = unwrap(Instr); Attribute Attr = Attribute::get(Call->getContext(), Name); - Call->addAttribute(Index, Attr); + AddAttribute(Call, Index, Attr); } +static inline void AddCallAttributes(CallBase *Call, unsigned Index, const AttrBuilder& B) { + AttributeList Attrs = Call->getAttributes(); +#if LLVM_VERSION_LT(14, 0) + Attrs = Attrs.addAttributes(Call->getContext(), Index, B); +#else + // TODO(durin42): we should probably surface the explicit functions to Rust + // instead of this switch statement? + switch (Index) { + case AttributeList::ReturnIndex: + Attrs = Attrs.addRetAttributes(Call->getContext(), B); + break; + case AttributeList::FunctionIndex: + Attrs = Attrs.addFnAttributes(Call->getContext(), B); + break; + default: + Attrs = Attrs.addParamAttributes(Call->getContext(), Index-AttributeList::FirstArgIndex, B); + } +#endif + Call->setAttributes(Attrs); +} extern "C" void LLVMRustAddAlignmentCallSiteAttr(LLVMValueRef Instr, unsigned Index, @@ -224,8 +263,7 @@ extern "C" void LLVMRustAddAlignmentCallSiteAttr(LLVMValueRef Instr, CallBase *Call = unwrap(Instr); AttrBuilder B; B.addAlignmentAttr(Bytes); - Call->setAttributes(Call->getAttributes().addAttributes( - Call->getContext(), Index, B)); + AddCallAttributes(Call, Index, B); } extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, @@ -234,8 +272,7 @@ extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, CallBase *Call = unwrap(Instr); AttrBuilder B; B.addDereferenceableAttr(Bytes); - Call->setAttributes(Call->getAttributes().addAttributes( - Call->getContext(), Index, B)); + AddCallAttributes(Call, Index, B); } extern "C" void LLVMRustAddDereferenceableOrNullCallSiteAttr(LLVMValueRef Instr, @@ -244,15 +281,14 @@ extern "C" void LLVMRustAddDereferenceableOrNullCallSiteAttr(LLVMValueRef Instr, CallBase *Call = unwrap(Instr); AttrBuilder B; B.addDereferenceableOrNullAttr(Bytes); - Call->setAttributes(Call->getAttributes().addAttributes( - Call->getContext(), Index, B)); + AddCallAttributes(Call, Index, B); } extern "C" void LLVMRustAddByValCallSiteAttr(LLVMValueRef Instr, unsigned Index, LLVMTypeRef Ty) { CallBase *Call = unwrap(Instr); Attribute Attr = Attribute::getWithByValType(Call->getContext(), unwrap(Ty)); - Call->addAttribute(Index, Attr); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddStructRetCallSiteAttr(LLVMValueRef Instr, unsigned Index, @@ -263,28 +299,28 @@ extern "C" void LLVMRustAddStructRetCallSiteAttr(LLVMValueRef Instr, unsigned In #else Attribute Attr = Attribute::get(Call->getContext(), Attribute::StructRet); #endif - Call->addAttribute(Index, Attr); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddFunctionAttribute(LLVMValueRef Fn, unsigned Index, LLVMRustAttribute RustAttr) { Function *A = unwrap(Fn); Attribute Attr = Attribute::get(A->getContext(), fromRust(RustAttr)); - A->addAttribute(Index, Attr); + AddAttribute(A, Index, Attr); } extern "C" void LLVMRustAddAlignmentAttr(LLVMValueRef Fn, unsigned Index, uint32_t Bytes) { Function *A = unwrap(Fn); - A->addAttribute(Index, Attribute::getWithAlignment( + AddAttribute(A, Index, Attribute::getWithAlignment( A->getContext(), llvm::Align(Bytes))); } extern "C" void LLVMRustAddDereferenceableAttr(LLVMValueRef Fn, unsigned Index, uint64_t Bytes) { Function *A = unwrap(Fn); - A->addAttribute(Index, Attribute::getWithDereferenceableBytes(A->getContext(), + AddAttribute(A, Index, Attribute::getWithDereferenceableBytes(A->getContext(), Bytes)); } @@ -292,7 +328,7 @@ extern "C" void LLVMRustAddDereferenceableOrNullAttr(LLVMValueRef Fn, unsigned Index, uint64_t Bytes) { Function *A = unwrap(Fn); - A->addAttribute(Index, Attribute::getWithDereferenceableOrNullBytes( + AddAttribute(A, Index, Attribute::getWithDereferenceableOrNullBytes( A->getContext(), Bytes)); } @@ -300,7 +336,7 @@ extern "C" void LLVMRustAddByValAttr(LLVMValueRef Fn, unsigned Index, LLVMTypeRef Ty) { Function *F = unwrap(Fn); Attribute Attr = Attribute::getWithByValType(F->getContext(), unwrap(Ty)); - F->addAttribute(Index, Attr); + AddAttribute(F, Index, Attr); } extern "C" void LLVMRustAddStructRetAttr(LLVMValueRef Fn, unsigned Index, @@ -311,7 +347,7 @@ extern "C" void LLVMRustAddStructRetAttr(LLVMValueRef Fn, unsigned Index, #else Attribute Attr = Attribute::get(F->getContext(), Attribute::StructRet); #endif - F->addAttribute(Index, Attr); + AddAttribute(F, Index, Attr); } extern "C" void LLVMRustAddFunctionAttrStringValue(LLVMValueRef Fn, @@ -319,7 +355,7 @@ extern "C" void LLVMRustAddFunctionAttrStringValue(LLVMValueRef Fn, const char *Name, const char *Value) { Function *F = unwrap(Fn); - F->addAttribute(Index, Attribute::get( + AddAttribute(F, Index, Attribute::get( F->getContext(), StringRef(Name), StringRef(Value))); } @@ -330,7 +366,23 @@ extern "C" void LLVMRustRemoveFunctionAttributes(LLVMValueRef Fn, Attribute Attr = Attribute::get(F->getContext(), fromRust(RustAttr)); AttrBuilder B(Attr); auto PAL = F->getAttributes(); - auto PALNew = PAL.removeAttributes(F->getContext(), Index, B); + AttributeList PALNew; +#if LLVM_VERSION_LT(14, 0) + PALNew = PAL.removeAttributes(F->getContext(), Index, B); +#else + // TODO(durin42): we should probably surface the explicit functions to Rust + // instead of this switch statement? + switch (Index) { + case AttributeList::ReturnIndex: + PALNew = PAL.removeRetAttributes(F->getContext(), B); + break; + case AttributeList::FunctionIndex: + PALNew = PAL.removeFnAttributes(F->getContext(), B); + break; + default: + PALNew = PAL.removeParamAttributes(F->getContext(), Index-AttributeList::FirstArgIndex, B); + } +#endif F->setAttributes(PALNew); } From 484b79b950b1077d1bbfe6c4edf3bfe070d820b4 Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Tue, 7 Sep 2021 16:15:02 -0400 Subject: [PATCH 20/23] RustWrapper: just use the *AtIndex funcs directly Otherwise we're kind of reimplementing the inverse of the well-named methods, and that's not a direction we want to go. --- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 39 ++----------------- 1 file changed, 3 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index b3143f14d29cc..3a7af73c87de3 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -207,18 +207,7 @@ template static inline void AddAttribute(T *t, unsigned Index, Attri #if LLVM_VERSION_LT(14, 0) t->addAttribute(Index, Attr); #else - // TODO(durin42): we should probably surface the explicit functions to Rust - // instead of this switch statement? - switch (Index) { - case AttributeList::ReturnIndex: - t->addRetAttr(Attr); - break; - case AttributeList::FunctionIndex: - t->addFnAttr(Attr); - break; - default: - t->addParamAttr(Index-AttributeList::FirstArgIndex, Attr); - } + t->addAttributeAtIndex(Index, Attr); #endif } @@ -241,18 +230,7 @@ static inline void AddCallAttributes(CallBase *Call, unsigned Index, const AttrB #if LLVM_VERSION_LT(14, 0) Attrs = Attrs.addAttributes(Call->getContext(), Index, B); #else - // TODO(durin42): we should probably surface the explicit functions to Rust - // instead of this switch statement? - switch (Index) { - case AttributeList::ReturnIndex: - Attrs = Attrs.addRetAttributes(Call->getContext(), B); - break; - case AttributeList::FunctionIndex: - Attrs = Attrs.addFnAttributes(Call->getContext(), B); - break; - default: - Attrs = Attrs.addParamAttributes(Call->getContext(), Index-AttributeList::FirstArgIndex, B); - } + Attrs = Attrs.addAttributesAtIndex(Call->getContext(), Index, B); #endif Call->setAttributes(Attrs); } @@ -370,18 +348,7 @@ extern "C" void LLVMRustRemoveFunctionAttributes(LLVMValueRef Fn, #if LLVM_VERSION_LT(14, 0) PALNew = PAL.removeAttributes(F->getContext(), Index, B); #else - // TODO(durin42): we should probably surface the explicit functions to Rust - // instead of this switch statement? - switch (Index) { - case AttributeList::ReturnIndex: - PALNew = PAL.removeRetAttributes(F->getContext(), B); - break; - case AttributeList::FunctionIndex: - PALNew = PAL.removeFnAttributes(F->getContext(), B); - break; - default: - PALNew = PAL.removeParamAttributes(F->getContext(), Index-AttributeList::FirstArgIndex, B); - } + PALNew = PAL.removeAttributesAtIndex(F->getContext(), Index, B); #endif F->setAttributes(PALNew); } From 4d045406d1ee4c011e476fbeae81976c8012e886 Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Wed, 8 Sep 2021 10:47:41 -0400 Subject: [PATCH 21/23] RustWrapper: remove some uses of AttrBuilder Turns out we can also use Attribute::get*() methods here, and avoid the AttrBuilder and an extra helper method here. --- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 3a7af73c87de3..9850f395a0f65 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -225,41 +225,28 @@ extern "C" void LLVMRustAddCallSiteAttrString(LLVMValueRef Instr, unsigned Index AddAttribute(Call, Index, Attr); } -static inline void AddCallAttributes(CallBase *Call, unsigned Index, const AttrBuilder& B) { - AttributeList Attrs = Call->getAttributes(); -#if LLVM_VERSION_LT(14, 0) - Attrs = Attrs.addAttributes(Call->getContext(), Index, B); -#else - Attrs = Attrs.addAttributesAtIndex(Call->getContext(), Index, B); -#endif - Call->setAttributes(Attrs); -} - extern "C" void LLVMRustAddAlignmentCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint32_t Bytes) { CallBase *Call = unwrap(Instr); - AttrBuilder B; - B.addAlignmentAttr(Bytes); - AddCallAttributes(Call, Index, B); + Attribute Attr = Attribute::getWithAlignment(Call->getContext(), Align(Bytes)); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint64_t Bytes) { CallBase *Call = unwrap(Instr); - AttrBuilder B; - B.addDereferenceableAttr(Bytes); - AddCallAttributes(Call, Index, B); + Attribute Attr = Attribute::getWithDereferenceableBytes(Call->getContext(), Bytes); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddDereferenceableOrNullCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint64_t Bytes) { CallBase *Call = unwrap(Instr); - AttrBuilder B; - B.addDereferenceableOrNullAttr(Bytes); - AddCallAttributes(Call, Index, B); + Attribute Attr = Attribute::getWithDereferenceableOrNullBytes(Call->getContext(), Bytes); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddByValCallSiteAttr(LLVMValueRef Instr, unsigned Index, From 85495981d8671f02970b0516297639a96600ed7c Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 26 Aug 2021 18:37:44 +0000 Subject: [PATCH 22/23] Improve error when an .rlib can't be parsed This usually describes either an error in the compiler itself or some sort of IO error. Either way, we should report it to the user rather than just saying "crate not found". This only gives an error if the crate couldn't be loaded at all - if the compiler finds another .rlib or .rmeta file which was valid, it will continue to compile the crate. Example output: ``` error[E0785]: found invalid metadata files for crate `foo` --> bar.rs:3:24 | 3 | println!("{}", foo::FOO_11_49[0]); | ^^^ | = warning: failed to parse rlib '/home/joshua/test-rustdoc/libfoo.rlib': Invalid archive extended name offset ``` --- compiler/rustc_error_codes/src/error_codes.rs | 1 + .../src/error_codes/E0786.md | 14 +++ compiler/rustc_metadata/src/locator.rs | 89 +++++++++++++++---- .../invalid-library/Makefile | 2 +- src/test/ui/crate-loading/auxiliary/libbar.so | 1 + .../ui/crate-loading/auxiliary/libfoo.rlib | 0 src/test/ui/crate-loading/invalid-rlib.rs | 5 ++ src/test/ui/crate-loading/invalid-rlib.stderr | 11 +++ src/test/ui/crate-loading/invalid-so.rs | 4 + src/test/ui/crate-loading/invalid-so.stderr | 11 +++ 10 files changed, 122 insertions(+), 16 deletions(-) create mode 100644 compiler/rustc_error_codes/src/error_codes/E0786.md create mode 100644 src/test/ui/crate-loading/auxiliary/libbar.so create mode 100644 src/test/ui/crate-loading/auxiliary/libfoo.rlib create mode 100644 src/test/ui/crate-loading/invalid-rlib.rs create mode 100644 src/test/ui/crate-loading/invalid-rlib.stderr create mode 100644 src/test/ui/crate-loading/invalid-so.rs create mode 100644 src/test/ui/crate-loading/invalid-so.stderr diff --git a/compiler/rustc_error_codes/src/error_codes.rs b/compiler/rustc_error_codes/src/error_codes.rs index 45d91c2047d41..556a65bacf2a0 100644 --- a/compiler/rustc_error_codes/src/error_codes.rs +++ b/compiler/rustc_error_codes/src/error_codes.rs @@ -481,6 +481,7 @@ E0782: include_str!("./error_codes/E0782.md"), E0783: include_str!("./error_codes/E0783.md"), E0784: include_str!("./error_codes/E0784.md"), E0785: include_str!("./error_codes/E0785.md"), +E0786: include_str!("./error_codes/E0786.md"), ; // E0006, // merged with E0005 // E0008, // cannot bind by-move into a pattern guard diff --git a/compiler/rustc_error_codes/src/error_codes/E0786.md b/compiler/rustc_error_codes/src/error_codes/E0786.md new file mode 100644 index 0000000000000..4a9635bf51637 --- /dev/null +++ b/compiler/rustc_error_codes/src/error_codes/E0786.md @@ -0,0 +1,14 @@ +A metadata file was invalid. + +Erroneous code example: + +```ignore (needs extern files) +use ::foo; // error: found invalid metadata files for crate `foo` +``` + +When loading crates, each crate must have a valid metadata file. +Invalid files could be caused by filesystem corruption, +an IO error while reading the file, or (rarely) a bug in the compiler itself. + +Consider deleting the file and recreating it, +or reporting a bug against the compiler. diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index d3512b6cf579e..ce568781f0283 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -349,6 +349,7 @@ impl<'a> CrateLocator<'a> { self.crate_rejections.via_kind.clear(); self.crate_rejections.via_version.clear(); self.crate_rejections.via_filename.clear(); + self.crate_rejections.via_invalid.clear(); } crate fn maybe_load_library_crate(&mut self) -> Result, CrateError> { @@ -538,7 +539,16 @@ impl<'a> CrateLocator<'a> { continue; } } - Err(err) => { + Err(MetadataError::LoadFailure(err)) => { + // The file was present and created by the same compiler version, but we + // couldn't load it for some reason. Give a hard error instead of silently + // ignoring it, but only if we would have given an error anyway. + self.crate_rejections + .via_invalid + .push(CrateMismatch { path: lib, got: err }); + continue; + } + Err(err @ MetadataError::NotPresent(_)) => { warn!("no metadata found: {}", err); continue; } @@ -716,25 +726,28 @@ impl<'a> CrateLocator<'a> { fn get_metadata_section( target: &Target, flavor: CrateFlavor, - filename: &Path, + filename: &'p Path, loader: &dyn MetadataLoader, -) -> Result { +) -> Result> { if !filename.exists() { - return Err(format!("no such file: '{}'", filename.display())); + return Err(MetadataError::NotPresent(filename)); } let raw_bytes: MetadataRef = match flavor { - CrateFlavor::Rlib => loader.get_rlib_metadata(target, filename)?, + CrateFlavor::Rlib => { + loader.get_rlib_metadata(target, filename).map_err(MetadataError::LoadFailure)? + } CrateFlavor::Dylib => { - let buf = loader.get_dylib_metadata(target, filename)?; + let buf = + loader.get_dylib_metadata(target, filename).map_err(MetadataError::LoadFailure)?; // The header is uncompressed let header_len = METADATA_HEADER.len(); debug!("checking {} bytes of metadata-version stamp", header_len); let header = &buf[..cmp::min(header_len, buf.len())]; if header != METADATA_HEADER { - return Err(format!( - "incompatible metadata version found: '{}'", + return Err(MetadataError::LoadFailure(format!( + "invalid metadata version found: {}", filename.display() - )); + ))); } // Header is okay -> inflate the actual metadata @@ -744,17 +757,28 @@ fn get_metadata_section( match FrameDecoder::new(compressed_bytes).read_to_end(&mut inflated) { Ok(_) => rustc_erase_owner!(OwningRef::new(inflated).map_owner_box()), Err(_) => { - return Err(format!("failed to decompress metadata: {}", filename.display())); + return Err(MetadataError::LoadFailure(format!( + "failed to decompress metadata: {}", + filename.display() + ))); } } } CrateFlavor::Rmeta => { // mmap the file, because only a small fraction of it is read. - let file = std::fs::File::open(filename) - .map_err(|_| format!("failed to open rmeta metadata: '{}'", filename.display()))?; + let file = std::fs::File::open(filename).map_err(|_| { + MetadataError::LoadFailure(format!( + "failed to open rmeta metadata: '{}'", + filename.display() + )) + })?; let mmap = unsafe { Mmap::map(file) }; - let mmap = mmap - .map_err(|_| format!("failed to mmap rmeta metadata: '{}'", filename.display()))?; + let mmap = mmap.map_err(|_| { + MetadataError::LoadFailure(format!( + "failed to mmap rmeta metadata: '{}'", + filename.display() + )) + })?; rustc_erase_owner!(OwningRef::new(mmap).map_owner_box()) } @@ -763,7 +787,10 @@ fn get_metadata_section( if blob.is_compatible() { Ok(blob) } else { - Err(format!("incompatible metadata version found: '{}'", filename.display())) + Err(MetadataError::LoadFailure(format!( + "invalid metadata version found: {}", + filename.display() + ))) } } @@ -842,6 +869,7 @@ struct CrateRejections { via_kind: Vec, via_version: Vec, via_filename: Vec, + via_invalid: Vec, } /// Candidate rejection reasons collected during crate search. @@ -871,6 +899,24 @@ crate enum CrateError { NonDylibPlugin(Symbol), } +enum MetadataError<'a> { + /// The file was missing. + NotPresent(&'a Path), + /// The file was present and invalid. + LoadFailure(String), +} + +impl fmt::Display for MetadataError<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MetadataError::NotPresent(filename) => { + f.write_str(&format!("no such file: '{}'", filename.display())) + } + MetadataError::LoadFailure(msg) => f.write_str(msg), + } + } +} + impl CrateError { crate fn report(self, sess: &Session, span: Span, missing_core: bool) -> ! { let mut err = match self { @@ -1044,6 +1090,19 @@ impl CrateError { } err.note(&msg); err + } else if !locator.crate_rejections.via_invalid.is_empty() { + let mut err = struct_span_err!( + sess, + span, + E0786, + "found invalid metadata files for crate `{}`{}", + crate_name, + add, + ); + for CrateMismatch { path: _, got } in locator.crate_rejections.via_invalid { + err.note(&got); + } + err } else { let mut err = struct_span_err!( sess, diff --git a/src/test/run-make-fulldeps/invalid-library/Makefile b/src/test/run-make-fulldeps/invalid-library/Makefile index c75713c3ee53d..de463a330149b 100644 --- a/src/test/run-make-fulldeps/invalid-library/Makefile +++ b/src/test/run-make-fulldeps/invalid-library/Makefile @@ -3,4 +3,4 @@ all: touch $(TMPDIR)/lib.rmeta $(AR) crus $(TMPDIR)/libfoo-ffffffff-1.0.rlib $(TMPDIR)/lib.rmeta - $(RUSTC) foo.rs 2>&1 | $(CGREP) "can't find crate for" + $(RUSTC) foo.rs 2>&1 | $(CGREP) "found invalid metadata" diff --git a/src/test/ui/crate-loading/auxiliary/libbar.so b/src/test/ui/crate-loading/auxiliary/libbar.so new file mode 100644 index 0000000000000..8b137891791fe --- /dev/null +++ b/src/test/ui/crate-loading/auxiliary/libbar.so @@ -0,0 +1 @@ + diff --git a/src/test/ui/crate-loading/auxiliary/libfoo.rlib b/src/test/ui/crate-loading/auxiliary/libfoo.rlib new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/src/test/ui/crate-loading/invalid-rlib.rs b/src/test/ui/crate-loading/invalid-rlib.rs new file mode 100644 index 0000000000000..ad2166ce99d08 --- /dev/null +++ b/src/test/ui/crate-loading/invalid-rlib.rs @@ -0,0 +1,5 @@ +// compile-flags: --crate-type lib --extern foo={{src-base}}/crate-loading/auxiliary/libfoo.rlib +// edition:2018 +#![no_std] +use ::foo; //~ ERROR invalid metadata files for crate `foo` +//~| NOTE memory map must have a non-zero length diff --git a/src/test/ui/crate-loading/invalid-rlib.stderr b/src/test/ui/crate-loading/invalid-rlib.stderr new file mode 100644 index 0000000000000..873b4bb713427 --- /dev/null +++ b/src/test/ui/crate-loading/invalid-rlib.stderr @@ -0,0 +1,11 @@ +error[E0786]: found invalid metadata files for crate `foo` + --> $DIR/invalid-rlib.rs:4:7 + | +LL | use ::foo; + | ^^^ + | + = note: failed to mmap file '$DIR/auxiliary/libfoo.rlib': memory map must have a non-zero length + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0786`. diff --git a/src/test/ui/crate-loading/invalid-so.rs b/src/test/ui/crate-loading/invalid-so.rs new file mode 100644 index 0000000000000..8e9941b8fc86d --- /dev/null +++ b/src/test/ui/crate-loading/invalid-so.rs @@ -0,0 +1,4 @@ +// compile-flags: --crate-type lib --extern bar={{src-base}}/crate-loading/auxiliary/libbar.so +// edition:2018 +use ::bar; //~ ERROR invalid metadata files for crate `bar` +//~| NOTE invalid metadata version diff --git a/src/test/ui/crate-loading/invalid-so.stderr b/src/test/ui/crate-loading/invalid-so.stderr new file mode 100644 index 0000000000000..dafc9457587e5 --- /dev/null +++ b/src/test/ui/crate-loading/invalid-so.stderr @@ -0,0 +1,11 @@ +error[E0786]: found invalid metadata files for crate `bar` + --> $DIR/invalid-so.rs:3:7 + | +LL | use ::bar; + | ^^^ + | + = note: invalid metadata version found: $DIR/auxiliary/libbar.so + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0786`. From c86c63436ac074d824d76e151c468554191e168e Mon Sep 17 00:00:00 2001 From: Hans Niklas Jacob Date: Wed, 8 Sep 2021 16:03:15 +0200 Subject: [PATCH 23/23] Allow missing code examples in trait impls. --- src/librustdoc/passes/doc_test_lints.rs | 20 +++++++++++++++++++ .../lint-missing-doc-code-example.rs | 7 +++++++ 2 files changed, 27 insertions(+) diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 03bc2b52f178f..1b5ec1b08fab0 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -10,6 +10,7 @@ use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString}; use crate::visit_ast::inherits_doc_hidden; +use rustc_hir as hir; use rustc_middle::lint::LintLevelSource; use rustc_session::lint; use rustc_span::symbol::sym; @@ -67,13 +68,32 @@ crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> boo | clean::ImportItem(_) | clean::PrimitiveItem(_) | clean::KeywordItem(_) + // check for trait impl + | clean::ImplItem(clean::Impl { trait_: Some(_), .. }) ) { return false; } + // The `expect_def_id()` should be okay because `local_def_id_to_hir_id` // would presumably panic if a fake `DefIndex` were passed. let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id.expect_def_id().expect_local()); + + // check if parent is trait impl + if let Some(parent_hir_id) = cx.tcx.hir().find_parent_node(hir_id) { + if let Some(parent_node) = cx.tcx.hir().find(parent_hir_id) { + if matches!( + parent_node, + hir::Node::Item(hir::Item { + kind: hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }), + .. + }) + ) { + return false; + } + } + } + if cx.tcx.hir().attrs(hir_id).lists(sym::doc).has_word(sym::hidden) || inherits_doc_hidden(cx.tcx, hir_id) { diff --git a/src/test/rustdoc-ui/lint-missing-doc-code-example.rs b/src/test/rustdoc-ui/lint-missing-doc-code-example.rs index 41e8847792694..7dd2ebfedbbd7 100644 --- a/src/test/rustdoc-ui/lint-missing-doc-code-example.rs +++ b/src/test/rustdoc-ui/lint-missing-doc-code-example.rs @@ -70,6 +70,13 @@ pub union Union { b: f32, } +// no code example and it's fine! +impl Clone for Struct { + fn clone(&self) -> Self { + Self { field: self.field } + } +} + #[doc(hidden)] pub mod foo {