Skip to content

Commit

Permalink
Auto merge of rust-lang#99242 - Dylan-DPC:rollup-34bqdh8, r=Dylan-DPC
Browse files Browse the repository at this point in the history
Rollup of 6 pull requests

Successful merges:

 - rust-lang#98072 (Add provider API to error trait)
 - rust-lang#98580 (Emit warning when named arguments are used positionally in format)
 - rust-lang#99000 (Move abstract const to middle)
 - rust-lang#99192 (Fix spans for asm diagnostics)
 - rust-lang#99222 (Better error message for generic_const_exprs inference failure)
 - rust-lang#99236 (solaris: unbreak build on native platform)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jul 14, 2022
2 parents 24699bc + f74358d commit 74621c7
Show file tree
Hide file tree
Showing 46 changed files with 1,320 additions and 778 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4514,6 +4514,7 @@ dependencies = [
"rustc_data_structures",
"rustc_errors",
"rustc_hir",
"rustc_index",
"rustc_infer",
"rustc_middle",
"rustc_session",
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_builtin_macros/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,8 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::Inl

let mut template_strs = Vec::with_capacity(args.templates.len());

for template_expr in args.templates.into_iter() {
if !template.is_empty() {
for (i, template_expr) in args.templates.into_iter().enumerate() {
if i != 0 {
template.push(ast::InlineAsmTemplatePiece::String("\n".to_string()));
}

Expand Down
85 changes: 72 additions & 13 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{InnerSpan, Span};
use smallvec::SmallVec;

use rustc_lint_defs::builtin::NAMED_ARGUMENTS_USED_POSITIONALLY;
use rustc_lint_defs::{BufferedEarlyLint, BuiltinLintDiagnostics, LintId};
use rustc_parse_format::{Count, FormatSpec};
use std::borrow::Cow;
use std::collections::hash_map::Entry;

Expand Down Expand Up @@ -57,7 +60,7 @@ struct Context<'a, 'b> {
/// Unique format specs seen for each argument.
arg_unique_types: Vec<Vec<ArgumentType>>,
/// Map from named arguments to their resolved indices.
names: FxHashMap<Symbol, usize>,
names: FxHashMap<Symbol, (usize, Span)>,

/// The latest consecutive literal strings, or empty if there weren't any.
literal: String,
Expand Down Expand Up @@ -130,9 +133,9 @@ fn parse_args<'a>(
ecx: &mut ExtCtxt<'a>,
sp: Span,
tts: TokenStream,
) -> PResult<'a, (P<ast::Expr>, Vec<P<ast::Expr>>, FxHashMap<Symbol, usize>)> {
) -> PResult<'a, (P<ast::Expr>, Vec<P<ast::Expr>>, FxHashMap<Symbol, (usize, Span)>)> {
let mut args = Vec::<P<ast::Expr>>::new();
let mut names = FxHashMap::<Symbol, usize>::default();
let mut names = FxHashMap::<Symbol, (usize, Span)>::default();

let mut p = ecx.new_parser_from_tts(tts);

Expand Down Expand Up @@ -197,7 +200,7 @@ fn parse_args<'a>(
p.bump();
p.expect(&token::Eq)?;
let e = p.parse_expr()?;
if let Some(prev) = names.get(&ident.name) {
if let Some((prev, _)) = names.get(&ident.name) {
ecx.struct_span_err(e.span, &format!("duplicate argument named `{}`", ident))
.span_label(args[*prev].span, "previously here")
.span_label(e.span, "duplicate argument")
Expand All @@ -210,7 +213,7 @@ fn parse_args<'a>(
// if the input is valid, we can simply append to the positional
// args. And remember the names.
let slot = args.len();
names.insert(ident.name, slot);
names.insert(ident.name, (slot, ident.span));
args.push(e);
}
_ => {
Expand All @@ -222,7 +225,7 @@ fn parse_args<'a>(
);
err.span_label(e.span, "positional arguments must be before named arguments");
for pos in names.values() {
err.span_label(args[*pos].span, "named argument");
err.span_label(args[pos.0].span, "named argument");
}
err.emit();
}
Expand All @@ -242,7 +245,8 @@ impl<'a, 'b> Context<'a, 'b> {
fn resolve_name_inplace(&self, p: &mut parse::Piece<'_>) {
// NOTE: the `unwrap_or` branch is needed in case of invalid format
// arguments, e.g., `format_args!("{foo}")`.
let lookup = |s: &str| *self.names.get(&Symbol::intern(s)).unwrap_or(&0);
let lookup =
|s: &str| self.names.get(&Symbol::intern(s)).unwrap_or(&(0, Span::default())).0;

match *p {
parse::String(_) => {}
Expand Down Expand Up @@ -548,7 +552,7 @@ impl<'a, 'b> Context<'a, 'b> {
match self.names.get(&name) {
Some(&idx) => {
// Treat as positional arg.
self.verify_arg_type(Capture(idx), ty)
self.verify_arg_type(Capture(idx.0), ty)
}
None => {
// For the moment capturing variables from format strings expanded from macros is
Expand All @@ -565,7 +569,7 @@ impl<'a, 'b> Context<'a, 'b> {
};
self.num_captured_args += 1;
self.args.push(self.ecx.expr_ident(span, Ident::new(name, span)));
self.names.insert(name, idx);
self.names.insert(name, (idx, span));
self.verify_arg_type(Capture(idx), ty)
} else {
let msg = format!("there is no argument named `{}`", name);
Expand Down Expand Up @@ -967,14 +971,57 @@ pub fn expand_format_args_nl<'cx>(
expand_format_args_impl(ecx, sp, tts, true)
}

fn lint_named_arguments_used_positionally(
names: FxHashMap<Symbol, (usize, Span)>,
cx: &mut Context<'_, '_>,
unverified_pieces: Vec<parse::Piece<'_>>,
) {
let mut used_argument_names = FxHashSet::<&str>::default();
for piece in unverified_pieces {
if let rustc_parse_format::Piece::NextArgument(a) = piece {
match a.position {
rustc_parse_format::Position::ArgumentNamed(arg_name, _) => {
used_argument_names.insert(arg_name);
}
_ => {}
};
match a.format {
FormatSpec { width: Count::CountIsName(s, _), .. }
| FormatSpec { precision: Count::CountIsName(s, _), .. } => {
used_argument_names.insert(s);
}
_ => {}
};
}
}

for (symbol, (index, span)) in names {
if !used_argument_names.contains(symbol.as_str()) {
let msg = format!("named argument `{}` is not used by name", symbol.as_str());
let arg_span = cx.arg_spans[index];
cx.ecx.buffered_early_lint.push(BufferedEarlyLint {
span: MultiSpan::from_span(span),
msg: msg.clone(),
node_id: ast::CRATE_NODE_ID,
lint_id: LintId::of(&NAMED_ARGUMENTS_USED_POSITIONALLY),
diagnostic: BuiltinLintDiagnostics::NamedArgumentUsedPositionally(
arg_span,
span,
symbol.to_string(),
),
});
}
}
}

/// Take the various parts of `format_args!(efmt, args..., name=names...)`
/// and construct the appropriate formatting expression.
pub fn expand_preparsed_format_args(
ecx: &mut ExtCtxt<'_>,
sp: Span,
efmt: P<ast::Expr>,
args: Vec<P<ast::Expr>>,
names: FxHashMap<Symbol, usize>,
names: FxHashMap<Symbol, (usize, Span)>,
append_newline: bool,
) -> P<ast::Expr> {
// NOTE: this verbose way of initializing `Vec<Vec<ArgumentType>>` is because
Expand Down Expand Up @@ -1073,7 +1120,12 @@ pub fn expand_preparsed_format_args(
.map(|span| fmt_span.from_inner(InnerSpan::new(span.start, span.end)))
.collect();

let named_pos: FxHashSet<usize> = names.values().cloned().collect();
let named_pos: FxHashSet<usize> = names.values().cloned().map(|(i, _)| i).collect();

// Clone `names` because `names` in Context get updated by verify_piece, which includes usages
// of the names of named arguments, resulting in incorrect errors if a name argument is used
// but not declared, such as: `println!("x = {x}");`
let named_arguments = names.clone();

let mut cx = Context {
ecx,
Expand Down Expand Up @@ -1101,9 +1153,11 @@ pub fn expand_preparsed_format_args(
is_literal: parser.is_literal,
};

// This needs to happen *after* the Parser has consumed all pieces to create all the spans
// This needs to happen *after* the Parser has consumed all pieces to create all the spans.
// unverified_pieces is used later to check named argument names are used, so clone each piece.
let pieces = unverified_pieces
.into_iter()
.iter()
.cloned()
.map(|mut piece| {
cx.verify_piece(&piece);
cx.resolve_name_inplace(&mut piece);
Expand Down Expand Up @@ -1265,6 +1319,11 @@ pub fn expand_preparsed_format_args(
}

diag.emit();
} else if cx.invalid_refs.is_empty() && !named_arguments.is_empty() {
// Only check for unused named argument names if there are no other errors to avoid causing
// too much noise in output errors, such as when a named argument is entirely unused.
// We also only need to perform this check if there are actually named arguments.
lint_named_arguments_used_positionally(named_arguments, &mut cx, unverified_pieces);
}

cx.into_expr()
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::{self, Lrc};
use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed, MultiSpan, PResult};
use rustc_lint_defs::builtin::PROC_MACRO_BACK_COMPAT;
use rustc_lint_defs::BuiltinLintDiagnostics;
use rustc_lint_defs::{BufferedEarlyLint, BuiltinLintDiagnostics};
use rustc_parse::{self, parser, MACRO_ARGUMENTS};
use rustc_session::{parse::ParseSess, Limit, Session, SessionDiagnostic};
use rustc_span::def_id::{CrateNum, DefId, LocalDefId};
Expand Down Expand Up @@ -988,6 +988,8 @@ pub struct ExtCtxt<'a> {
pub expansions: FxHashMap<Span, Vec<String>>,
/// Used for running pre-expansion lints on freshly loaded modules.
pub(super) lint_store: LintStoreExpandDyn<'a>,
/// Used for storing lints generated during expansion, like `NAMED_ARGUMENTS_USED_POSITIONALLY`
pub buffered_early_lint: Vec<BufferedEarlyLint>,
/// When we 'expand' an inert attribute, we leave it
/// in the AST, but insert it here so that we know
/// not to expand it again.
Expand Down Expand Up @@ -1020,6 +1022,7 @@ impl<'a> ExtCtxt<'a> {
force_mode: false,
expansions: FxHashMap::default(),
expanded_inert_attrs: MarkedAttrs::new(),
buffered_early_lint: vec![],
}
}

Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use rustc_middle::infer::unify_key::{ConstVarValue, ConstVariableValue};
use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind, ToType};
use rustc_middle::mir::interpret::{ErrorHandled, EvalToValTreeResult};
use rustc_middle::traits::select;
use rustc_middle::ty::abstract_const::AbstractConst;
use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable};
use rustc_middle::ty::relate::RelateResult;
Expand Down Expand Up @@ -1651,14 +1652,18 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
unevaluated: ty::Unevaluated<'tcx>,
span: Option<Span>,
) -> EvalToValTreeResult<'tcx> {
let substs = self.resolve_vars_if_possible(unevaluated.substs);
let mut substs = self.resolve_vars_if_possible(unevaluated.substs);
debug!(?substs);

// Postpone the evaluation of constants whose substs depend on inference
// variables
if substs.has_infer_types_or_consts() {
debug!("substs have infer types or consts: {:?}", substs);
return Err(ErrorHandled::TooGeneric);
let ac = AbstractConst::new(self.tcx, unevaluated.shrink());
if let Ok(None) = ac {
substs = InternalSubsts::identity_for_item(self.tcx, unevaluated.def.did);
} else {
return Err(ErrorHandled::TooGeneric);
}
}

let param_env_erased = self.tcx.erase_regions(param_env);
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan, PResult};
use rustc_expand::base::{ExtCtxt, LintStoreExpand, ResolverExpand};
use rustc_hir::def_id::{StableCrateId, LOCAL_CRATE};
use rustc_hir::definitions::Definitions;
use rustc_lint::{EarlyCheckNode, LintStore};
use rustc_lint::{BufferedEarlyLint, EarlyCheckNode, LintStore};
use rustc_metadata::creader::CStore;
use rustc_metadata::{encode_metadata, EncodedMetadata};
use rustc_middle::arena::Arena;
Expand Down Expand Up @@ -336,12 +336,15 @@ pub fn configure_and_expand(

let lint_store = LintStoreExpandImpl(lint_store);
let mut ecx = ExtCtxt::new(sess, cfg, resolver, Some(&lint_store));

// Expand macros now!
let krate = sess.time("expand_crate", || ecx.monotonic_expander().expand_crate(krate));

// The rest is error reporting

sess.parse_sess.buffered_lints.with_lock(|buffered_lints: &mut Vec<BufferedEarlyLint>| {
buffered_lints.append(&mut ecx.buffered_early_lint);
});

sess.time("check_unused_macros", || {
ecx.check_unused_macros();
});
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,18 @@ pub trait LintContext: Sized {
Applicability::MachineApplicable,
);
},
BuiltinLintDiagnostics::NamedArgumentUsedPositionally(positional_arg, named_arg, name) => {
db.span_label(named_arg, "this named argument is only referred to by position in formatting string");
let msg = format!("this formatting argument uses named argument `{}` by position", name);
db.span_label(positional_arg, msg);
db.span_suggestion_verbose(
positional_arg,
"use the named argument by name to avoid ambiguity",
format!("{{{}}}", name),
Applicability::MaybeIncorrect,
);

}
}
// Rewrap `db`, and pass control to the user.
decorate(LintDiagnosticBuilder::new(db));
Expand Down
31 changes: 31 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3292,6 +3292,7 @@ declare_lint_pass! {
TEST_UNSTABLE_LINT,
FFI_UNWIND_CALLS,
REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS,
NAMED_ARGUMENTS_USED_POSITIONALLY,
]
}

Expand Down Expand Up @@ -3996,3 +3997,33 @@ declare_lint! {
"call to foreign functions or function pointers with FFI-unwind ABI",
@feature_gate = sym::c_unwind;
}

declare_lint! {
/// The `named_arguments_used_positionally` lint detects cases where named arguments are only
/// used positionally in format strings. This usage is valid but potentially very confusing.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(named_arguments_used_positionally)]
/// fn main() {
/// let _x = 5;
/// println!("{}", _x = 1); // Prints 1, will trigger lint
///
/// println!("{}", _x); // Prints 5, no lint emitted
/// println!("{_x}", _x = _x); // Prints 5, no lint emitted
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Rust formatting strings can refer to named arguments by their position, but this usage is
/// potentially confusing. In particular, readers can incorrectly assume that the declaration
/// of named arguments is an assignment (which would produce the unit type).
/// For backwards compatibility, this is not a hard error.
pub NAMED_ARGUMENTS_USED_POSITIONALLY,
Warn,
"named arguments in format used positionally"
}
1 change: 1 addition & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ pub enum BuiltinLintDiagnostics {
/// If true, the lifetime will be fully elided.
use_span: Option<(Span, bool)>,
},
NamedArgumentUsedPositionally(Span, Span, String),
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::metadata::ModChild;
use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
use rustc_middle::mir::interpret::{AllocDecodingSession, AllocDecodingState};
use rustc_middle::thir;
use rustc_middle::ty::codec::TyDecoder;
use rustc_middle::ty::fast_reject::SimplifiedType;
use rustc_middle::ty::GeneratorDiagnosticData;
Expand Down Expand Up @@ -638,7 +637,7 @@ impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for Span {
}
}

impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for &'tcx [thir::abstract_const::Node<'tcx>] {
impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for &'tcx [ty::abstract_const::Node<'tcx>] {
fn decode(d: &mut DecodeContext<'a, 'tcx>) -> Self {
ty::codec::RefDecodable::decode(d)
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use rustc_middle::metadata::ModChild;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
use rustc_middle::mir;
use rustc_middle::thir;
use rustc_middle::ty::fast_reject::SimplifiedType;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, ReprOptions, Ty};
Expand Down Expand Up @@ -361,7 +360,7 @@ define_tables! {
mir_for_ctfe: Table<DefIndex, LazyValue<mir::Body<'static>>>,
promoted_mir: Table<DefIndex, LazyValue<IndexVec<mir::Promoted, mir::Body<'static>>>>,
// FIXME(compiler-errors): Why isn't this a LazyArray?
thir_abstract_const: Table<DefIndex, LazyValue<&'static [thir::abstract_const::Node<'static>]>>,
thir_abstract_const: Table<DefIndex, LazyValue<&'static [ty::abstract_const::Node<'static>]>>,
impl_parent: Table<DefIndex, RawDefId>,
impl_polarity: Table<DefIndex, ty::ImplPolarity>,
constness: Table<DefIndex, hir::Constness>,
Expand Down
Loading

0 comments on commit 74621c7

Please sign in to comment.