1
Fork 0

Auto merge of #103431 - Dylan-DPC:rollup-oozfo89, r=Dylan-DPC

Rollup of 6 pull requests

Successful merges:

 - #101293 (Recover when unclosed char literal is parsed as a lifetime in some positions)
 - #101908 (Suggest let for assignment, and some code refactor)
 - #103192 (rustdoc: Eliminate uses of `EarlyDocLinkResolver::all_traits`)
 - #103226 (Check `needs_infer` before `needs_drop` during HIR generator analysis)
 - #103249 (resolve: Revert "Set effective visibilities for imports more precisely")
 - #103305 (Move some tests to more reasonable places)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
This commit is contained in:
bors 2022-10-23 11:33:18 +00:00
commit 9be2f35a4c
36 changed files with 542 additions and 265 deletions

View file

@ -463,6 +463,9 @@ pub enum StashKey {
UnderscoreForArrayLengths,
EarlySyntaxWarning,
CallIntoMethod,
/// When an invalid lifetime e.g. `'2` should be reinterpreted
/// as a char literal in the parser
LifetimeIsChar,
}
fn default_track_diagnostic(_: &Diagnostic) {}

View file

@ -6,8 +6,11 @@ use crate::{
use hir::{def_id::DefId, Body, HirId, HirIdMap};
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_middle::hir::place::{PlaceBase, Projection, ProjectionKind};
use rustc_middle::ty::{ParamEnv, TyCtxt};
use rustc_middle::{
hir::place::{PlaceBase, Projection, ProjectionKind},
ty::TypeVisitable,
};
pub(super) fn find_consumed_and_borrowed<'a, 'tcx>(
fcx: &'a FnCtxt<'a, 'tcx>,
@ -198,11 +201,13 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
// If the type being assigned needs dropped, then the mutation counts as a borrow
// since it is essentially doing `Drop::drop(&mut x); x = new_value;`.
//
// FIXME(drop-tracking): We need to be more responsible about inference
// variables here, since `needs_drop` is a "raw" type query, i.e. it
// basically requires types to have been fully resolved.
if assignee_place.place.base_ty.needs_drop(self.tcx, self.param_env) {
let ty = self.tcx.erase_regions(assignee_place.place.base_ty);
if ty.needs_infer() {
self.tcx.sess.delay_span_bug(
self.tcx.hir().span(assignee_place.hir_id),
&format!("inference variables in {ty}"),
);
} else if ty.needs_drop(self.tcx, self.param_env) {
self.places
.borrowed
.insert(TrackedValue::from_place_with_projections_allowed(assignee_place));

View file

@ -377,15 +377,6 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
debug!("is_borrowed_temporary: {:?}", self.drop_ranges.is_borrowed_temporary(expr));
let ty = self.fcx.typeck_results.borrow().expr_ty_adjusted_opt(expr);
let may_need_drop = |ty: Ty<'tcx>| {
// Avoid ICEs in needs_drop.
let ty = self.fcx.resolve_vars_if_possible(ty);
let ty = self.fcx.tcx.erase_regions(ty);
if ty.needs_infer() {
return true;
}
ty.needs_drop(self.fcx.tcx, self.fcx.param_env)
};
// Typically, the value produced by an expression is consumed by its parent in some way,
// so we only have to check if the parent contains a yield (note that the parent may, for
@ -403,9 +394,18 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
// src/test/ui/generator/drop-tracking-parent-expression.rs.
let scope = if self.drop_ranges.is_borrowed_temporary(expr)
|| ty.map_or(true, |ty| {
let needs_drop = may_need_drop(ty);
debug!(?needs_drop, ?ty);
needs_drop
// Avoid ICEs in needs_drop.
let ty = self.fcx.resolve_vars_if_possible(ty);
let ty = self.fcx.tcx.erase_regions(ty);
if ty.needs_infer() {
self.fcx
.tcx
.sess
.delay_span_bug(expr.span, &format!("inference variables in {ty}"));
true
} else {
ty.needs_drop(self.fcx.tcx, self.fcx.param_env)
}
}) {
self.rvalue_scopes.temporary_scope(self.region_scope_tree, expr.hir_id.local_id)
} else {

View file

@ -587,11 +587,6 @@ impl CStore {
self.get_crate_data(cnum).get_proc_macro_quoted_span(id, sess)
}
/// Decodes all traits in the crate (for rustdoc).
pub fn traits_in_crate_untracked(&self, cnum: CrateNum) -> impl Iterator<Item = DefId> + '_ {
self.get_crate_data(cnum).get_traits()
}
/// Decodes all trait impls in the crate (for rustdoc).
pub fn trait_impls_in_crate_untracked(
&self,

View file

@ -3,7 +3,9 @@ use rustc_ast::ast::{self, AttrStyle};
use rustc_ast::token::{self, CommentKind, Delimiter, Token, TokenKind};
use rustc_ast::tokenstream::TokenStream;
use rustc_ast::util::unicode::contains_text_flow_control_chars;
use rustc_errors::{error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, PResult};
use rustc_errors::{
error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, PResult, StashKey,
};
use rustc_lexer::unescape::{self, Mode};
use rustc_lexer::Cursor;
use rustc_lexer::{Base, DocStyle, RawStrError};
@ -203,7 +205,10 @@ impl<'a> StringReader<'a> {
// this is necessary.
let lifetime_name = self.str_from(start);
if starts_with_number {
self.err_span_(start, self.pos, "lifetimes cannot start with a number");
let span = self.mk_sp(start, self.pos);
let mut diag = self.sess.struct_err("lifetimes cannot start with a number");
diag.set_span(span);
diag.stash(span, StashKey::LifetimeIsChar);
}
let ident = Symbol::intern(lifetime_name);
token::Lifetime(ident)

View file

@ -42,8 +42,10 @@ use rustc_ast::{AnonConst, BinOp, BinOpKind, FnDecl, FnRetTy, MacCall, Param, Ty
use rustc_ast::{Arm, Async, BlockCheckMode, Expr, ExprKind, Label, Movability, RangeLimits};
use rustc_ast::{ClosureBinder, StmtKind};
use rustc_ast_pretty::pprust;
use rustc_errors::IntoDiagnostic;
use rustc_errors::{Applicability, Diagnostic, PResult};
use rustc_errors::{
Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, IntoDiagnostic, PResult,
StashKey,
};
use rustc_session::errors::ExprParenthesesNeeded;
use rustc_session::lint::builtin::BREAK_WITH_LABEL_AND_LOOP;
use rustc_session::lint::BuiltinLintDiagnostics;
@ -1513,11 +1515,11 @@ impl<'a> Parser<'a> {
/// Parse `'label: $expr`. The label is already parsed.
fn parse_labeled_expr(
&mut self,
label: Label,
label_: Label,
mut consume_colon: bool,
) -> PResult<'a, P<Expr>> {
let lo = label.ident.span;
let label = Some(label);
let lo = label_.ident.span;
let label = Some(label_);
let ate_colon = self.eat(&token::Colon);
let expr = if self.eat_keyword(kw::While) {
self.parse_while_expr(label, lo)
@ -1529,6 +1531,19 @@ impl<'a> Parser<'a> {
|| self.token.is_whole_block()
{
self.parse_block_expr(label, lo, BlockCheckMode::Default)
} else if !ate_colon
&& (matches!(self.token.kind, token::CloseDelim(_) | token::Comma)
|| self.token.is_op())
{
let lit = self.recover_unclosed_char(label_.ident, |self_| {
self_.sess.create_err(UnexpectedTokenAfterLabel {
span: self_.token.span,
remove_label: None,
enclose_in_block: None,
})
});
consume_colon = false;
Ok(self.mk_expr(lo, ExprKind::Lit(lit)))
} else if !ate_colon
&& (self.check_noexpect(&TokenKind::Comma) || self.check_noexpect(&TokenKind::Gt))
{
@ -1603,6 +1618,39 @@ impl<'a> Parser<'a> {
Ok(expr)
}
/// Emit an error when a char is parsed as a lifetime because of a missing quote
pub(super) fn recover_unclosed_char(
&mut self,
lifetime: Ident,
err: impl FnOnce(&mut Self) -> DiagnosticBuilder<'a, ErrorGuaranteed>,
) -> ast::Lit {
if let Some(mut diag) =
self.sess.span_diagnostic.steal_diagnostic(lifetime.span, StashKey::LifetimeIsChar)
{
diag.span_suggestion_verbose(
lifetime.span.shrink_to_hi(),
"add `'` to close the char literal",
"'",
Applicability::MaybeIncorrect,
)
.emit();
} else {
err(self)
.span_suggestion_verbose(
lifetime.span.shrink_to_hi(),
"add `'` to close the char literal",
"'",
Applicability::MaybeIncorrect,
)
.emit();
}
ast::Lit {
token_lit: token::Lit::new(token::LitKind::Char, lifetime.name, None),
kind: ast::LitKind::Char(lifetime.name.as_str().chars().next().unwrap_or('_')),
span: lifetime.span,
}
}
/// Recover on the syntax `do catch { ... }` suggesting `try { ... }` instead.
fn recover_do_catch(&mut self) -> PResult<'a, P<Expr>> {
let lo = self.token.span;
@ -1728,7 +1776,7 @@ impl<'a> Parser<'a> {
}
pub(super) fn parse_lit(&mut self) -> PResult<'a, Lit> {
self.parse_opt_lit().ok_or_else(|| {
self.parse_opt_lit().ok_or(()).or_else(|()| {
if let token::Interpolated(inner) = &self.token.kind {
let expr = match inner.as_ref() {
token::NtExpr(expr) => Some(expr),
@ -1740,12 +1788,22 @@ impl<'a> Parser<'a> {
let mut err = InvalidInterpolatedExpression { span: self.token.span }
.into_diagnostic(&self.sess.span_diagnostic);
err.downgrade_to_delayed_bug();
return err;
return Err(err);
}
}
}
let msg = format!("unexpected token: {}", super::token_descr(&self.token));
self.struct_span_err(self.token.span, &msg)
let token = self.token.clone();
let err = |self_: &mut Self| {
let msg = format!("unexpected token: {}", super::token_descr(&token));
self_.struct_span_err(token.span, &msg)
};
// On an error path, eagerly consider a lifetime to be an unclosed character lit
if self.token.is_lifetime() {
let lt = self.expect_lifetime();
Ok(self.recover_unclosed_char(lt.ident, err))
} else {
Err(err(self))
}
})
}

View file

@ -402,6 +402,25 @@ impl<'a> Parser<'a> {
} else {
PatKind::Path(qself, path)
}
} else if matches!(self.token.kind, token::Lifetime(_))
// In pattern position, we're totally fine with using "next token isn't colon"
// as a heuristic. We could probably just always try to recover if it's a lifetime,
// because we never have `'a: label {}` in a pattern position anyways, but it does
// keep us from suggesting something like `let 'a: Ty = ..` => `let 'a': Ty = ..`
&& !self.look_ahead(1, |token| matches!(token.kind, token::Colon))
{
// Recover a `'a` as a `'a'` literal
let lt = self.expect_lifetime();
let lit = self.recover_unclosed_char(lt.ident, |self_| {
let expected = expected.unwrap_or("pattern");
let msg =
format!("expected {}, found {}", expected, super::token_descr(&self_.token));
let mut err = self_.struct_span_err(self_.token.span, &msg);
err.span_label(self_.token.span, format!("expected {}", expected));
err
});
PatKind::Lit(self.mk_expr(lo, ExprKind::Lit(lit)))
} else {
// Try to parse everything else as literal with optional minus
match self.parse_literal_maybe_minus() {
@ -799,6 +818,7 @@ impl<'a> Parser<'a> {
|| t.kind == token::Dot // e.g. `.5` for recovery;
|| t.can_begin_literal_maybe_minus() // e.g. `42`.
|| t.is_whole_expr()
|| t.is_lifetime() // recover `'a` instead of `'a'`
})
}

View file

@ -1,5 +1,4 @@
use crate::NameBindingKind;
use crate::Resolver;
use crate::{ImportKind, NameBindingKind, Resolver};
use rustc_ast::ast;
use rustc_ast::visit;
use rustc_ast::visit::Visitor;
@ -45,14 +44,13 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
let module = self.r.get_module(module_id.to_def_id()).unwrap();
let resolutions = self.r.resolutions(module);
for (key, name_resolution) in resolutions.borrow().iter() {
for (_, name_resolution) in resolutions.borrow().iter() {
if let Some(mut binding) = name_resolution.borrow().binding() && !binding.is_ambiguity() {
// Set the given binding access level to `AccessLevel::Public` and
// sets the rest of the `use` chain to `AccessLevel::Exported` until
// we hit the actual exported item.
// FIXME: tag and is_public() condition must be deleted,
// but assertion fail occurs in import_id_for_ns
// FIXME: tag and is_public() condition should be removed, but assertions occur.
let tag = if binding.is_import() { AccessLevel::Exported } else { AccessLevel::Public };
if binding.vis.is_public() {
let mut prev_parent_id = module_id;
@ -60,16 +58,26 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
while let NameBindingKind::Import { binding: nested_binding, import, .. } =
binding.kind
{
let id = self.r.local_def_id(self.r.import_id_for_ns(import, key.ns));
self.update(
id,
let mut update = |node_id| self.update(
self.r.local_def_id(node_id),
binding.vis.expect_local(),
prev_parent_id,
level,
);
// In theory all the import IDs have individual visibilities and effective
// visibilities, but in practice these IDs go straigth to HIR where all
// their few uses assume that their (effective) visibility applies to the
// whole syntactic `use` item. So we update them all to the maximum value
// among the potential individual effective visibilities. Maybe HIR for
// imports shouldn't use three IDs at all.
update(import.id);
if let ImportKind::Single { additional_ids, .. } = import.kind {
update(additional_ids.0);
update(additional_ids.1);
}
level = AccessLevel::Exported;
prev_parent_id = id;
prev_parent_id = self.r.local_def_id(import.id);
binding = nested_binding;
}
}

View file

@ -2,7 +2,7 @@
use crate::diagnostics::{import_candidates, Suggestion};
use crate::Determinacy::{self, *};
use crate::Namespace::{self, *};
use crate::Namespace::*;
use crate::{module_to_string, names_to_string, ImportSuggestion};
use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment};
use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet};
@ -371,31 +371,6 @@ impl<'a> Resolver<'a> {
self.used_imports.insert(import.id);
}
}
/// Take primary and additional node IDs from an import and select one that corresponds to the
/// given namespace. The logic must match the corresponding logic from `fn lower_use_tree` that
/// assigns resolutons to IDs.
pub(crate) fn import_id_for_ns(&self, import: &Import<'_>, ns: Namespace) -> NodeId {
if let ImportKind::Single { additional_ids: (id1, id2), .. } = import.kind {
if let Some(resolutions) = self.import_res_map.get(&import.id) {
assert!(resolutions[ns].is_some(), "incorrectly finalized import");
return match ns {
TypeNS => import.id,
ValueNS => match resolutions.type_ns {
Some(_) => id1,
None => import.id,
},
MacroNS => match (resolutions.type_ns, resolutions.value_ns) {
(Some(_), Some(_)) => id2,
(Some(_), None) | (None, Some(_)) => id1,
(None, None) => import.id,
},
};
}
}
import.id
}
}
/// An error that may be transformed into a diagnostic later. Used to combine multiple unresolved

View file

@ -524,6 +524,9 @@ struct DiagnosticMetadata<'ast> {
/// Used to detect possible `if let` written without `let` and to provide structured suggestion.
in_if_condition: Option<&'ast Expr>,
/// Used to detect possible new binding written without `let` and to provide structured suggestion.
in_assignment: Option<&'ast Expr>,
/// If we are currently in a trait object definition. Used to point at the bounds when
/// encountering a struct or enum.
current_trait_object: Option<&'ast [ast::GenericBound]>,
@ -3905,6 +3908,11 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
self.resolve_expr(elem, Some(expr));
self.visit_expr(idx);
}
ExprKind::Assign(..) => {
let old = self.diagnostic_metadata.in_assignment.replace(expr);
visit::walk_expr(self, expr);
self.diagnostic_metadata.in_assignment = old;
}
_ => {
visit::walk_expr(self, expr);
}

View file

@ -708,7 +708,9 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
// If the trait has a single item (which wasn't matched by Levenshtein), suggest it
let suggestion = self.get_single_associated_item(&path, &source, is_expected);
self.r.add_typo_suggestion(err, suggestion, ident_span);
if !self.r.add_typo_suggestion(err, suggestion, ident_span) {
fallback = !self.let_binding_suggestion(err, ident_span);
}
}
fallback
}
@ -1105,41 +1107,14 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
// where a brace being opened means a block is being started. Look
// ahead for the next text to see if `span` is followed by a `{`.
let sm = self.r.session.source_map();
let mut sp = span;
loop {
sp = sm.next_point(sp);
match sm.span_to_snippet(sp) {
Ok(ref snippet) => {
if snippet.chars().any(|c| !c.is_whitespace()) {
break;
}
}
_ => break,
}
}
let sp = sm.span_look_ahead(span, None, Some(50));
let followed_by_brace = matches!(sm.span_to_snippet(sp), Ok(ref snippet) if snippet == "{");
// In case this could be a struct literal that needs to be surrounded
// by parentheses, find the appropriate span.
let mut i = 0;
let mut closing_brace = None;
loop {
sp = sm.next_point(sp);
match sm.span_to_snippet(sp) {
Ok(ref snippet) => {
if snippet == "}" {
closing_brace = Some(span.to(sp));
break;
}
}
_ => break,
}
i += 1;
// The bigger the span, the more likely we're incorrect --
// bound it to 100 chars long.
if i > 100 {
break;
}
}
let closing_span = sm.span_look_ahead(span, Some("}"), Some(50));
let closing_brace: Option<Span> = sm
.span_to_snippet(closing_span)
.map_or(None, |s| if s == "}" { Some(span.to(closing_span)) } else { None });
(followed_by_brace, closing_brace)
}
@ -1779,26 +1754,16 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
}
}
if let Ok(base_snippet) = base_snippet {
let mut sp = after_colon_sp;
for _ in 0..100 {
// Try to find an assignment
sp = sm.next_point(sp);
let snippet = sm.span_to_snippet(sp);
match snippet {
Ok(ref x) if x.as_str() == "=" => {
err.span_suggestion(
base_span,
"maybe you meant to write an assignment here",
format!("let {}", base_snippet),
Applicability::MaybeIncorrect,
);
show_label = false;
break;
}
Ok(ref x) if x.as_str() == "\n" => break,
Err(_) => break,
Ok(_) => {}
}
// Try to find an assignment
let eq_span = sm.span_look_ahead(after_colon_sp, Some("="), Some(50));
if let Ok(ref snippet) = sm.span_to_snippet(eq_span) && snippet == "=" {
err.span_suggestion(
base_span,
"maybe you meant to write an assignment here",
format!("let {}", base_snippet),
Applicability::MaybeIncorrect,
);
show_label = false;
}
}
}
@ -1815,6 +1780,31 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
false
}
fn let_binding_suggestion(&self, err: &mut Diagnostic, ident_span: Span) -> bool {
// try to give a suggestion for this pattern: `name = 1`, which is common in other languages
let mut added_suggestion = false;
if let Some(Expr { kind: ExprKind::Assign(lhs, _rhs, _), .. }) = self.diagnostic_metadata.in_assignment &&
let ast::ExprKind::Path(None, _) = lhs.kind {
let sm = self.r.session.source_map();
let line_span = sm.span_extend_to_line(ident_span);
let ident_name = sm.span_to_snippet(ident_span).unwrap();
// HACK(chenyukang): make sure ident_name is at the starting of the line to protect against macros
if sm
.span_to_snippet(line_span)
.map_or(false, |s| s.trim().starts_with(&ident_name))
{
err.span_suggestion_verbose(
ident_span.shrink_to_lo(),
"you might have meant to introduce a new binding",
"let ".to_string(),
Applicability::MaybeIncorrect,
);
added_suggestion = true;
}
}
added_suggestion
}
fn find_module(&mut self, def_id: DefId) -> Option<(Module<'a>, ImportSuggestion)> {
let mut result = None;
let mut seen_modules = FxHashSet::default();

View file

@ -877,6 +877,26 @@ impl SourceMap {
Span::new(BytePos(start_of_next_point), end_of_next_point, sp.ctxt(), None)
}
/// Returns a new span to check next none-whitespace character or some specified expected character
/// If `expect` is none, the first span of non-whitespace character is returned.
/// If `expect` presented, the first span of the character `expect` is returned
/// Otherwise, the span reached to limit is returned.
pub fn span_look_ahead(&self, span: Span, expect: Option<&str>, limit: Option<usize>) -> Span {
let mut sp = span;
for _ in 0..limit.unwrap_or(100 as usize) {
sp = self.next_point(sp);
if let Ok(ref snippet) = self.span_to_snippet(sp) {
if expect.map_or(false, |es| snippet == es) {
break;
}
if expect.is_none() && snippet.chars().any(|c| !c.is_whitespace()) {
break;
}
}
}
sp
}
/// Finds the width of the character, either before or after the end of provided span,
/// depending on the `forwards` parameter.
fn find_width_of_character_at_span(&self, sp: Span, forwards: bool) -> u32 {