Skip to content

Commit

Permalink
Auto merge of #100082 - matthiaskrgr:rollup-ywu4iux, r=matthiaskrgr
Browse files Browse the repository at this point in the history 
Rollup of 6 pull requests

Successful merges:

 - #99933 (parallelize HTML checking tool)
 - #99958 (Improve position named arguments lint underline and formatting names)
 - #100008 (Update all pre-cloned submodules on startup)
 - #100049 (:arrow_up: rust-analyzer)
 - #100070 (Clarify Cargo.toml comments)
 - #100074 (rustc-docs: Be less specific about the representation of `+bundle`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
@bors
bors committed Aug 3, 2022
2 parents b759b2e + e20f7f8 commit e141246
Show file tree
Hide file tree
Showing 59 changed files with 1,103 additions and 423 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,7 @@ dependencies = [
name = "html-checker"
version = "0.1.0"
dependencies = [
"rayon",
"walkdir",
]

Expand Down
84 changes: 59 additions & 25 deletions compiler/rustc_builtin_macros/src/format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use smallvec::SmallVec;

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

Expand Down Expand Up @@ -57,26 +58,45 @@ struct PositionalNamedArg {
replacement: Symbol,
/// The span for the positional named argument (so the lint can point a message to it)
positional_named_arg_span: Span,
has_formatting: bool,
}

impl PositionalNamedArg {
/// Determines what span to replace with the name of the named argument
fn get_span_to_replace(&self, cx: &Context<'_, '_>) -> Option<Span> {
/// Determines:
/// 1) span to be replaced with the name of the named argument and
/// 2) span to be underlined for error messages
fn get_positional_arg_spans(&self, cx: &Context<'_, '_>) -> (Option<Span>, Option<Span>) {
if let Some(inner_span) = &self.inner_span_to_replace {
return Some(
cx.fmtsp.from_inner(InnerSpan { start: inner_span.start, end: inner_span.end }),
);
let span =
cx.fmtsp.from_inner(InnerSpan { start: inner_span.start, end: inner_span.end });
(Some(span), Some(span))
} else if self.ty == PositionalNamedArgType::Arg {
// In the case of a named argument whose position is implicit, there will not be a span
// to replace. Instead, we insert the name after the `{`, which is the first character
// of arg_span.
return cx
.arg_spans
.get(self.cur_piece)
.map(|arg_span| arg_span.with_lo(arg_span.lo() + BytePos(1)).shrink_to_lo());
// In the case of a named argument whose position is implicit, if the argument *has*
// formatting, there will not be a span to replace. Instead, we insert the name after
// the `{`, which will be the first character of arg_span. If the argument does *not*
// have formatting, there may or may not be a span to replace. This is because
// whitespace is allowed in arguments without formatting (such as `format!("{ }", 1);`)
// but is not allowed in arguments with formatting (an error will be generated in cases
// like `format!("{ :1.1}", 1.0f32);`.
// For the message span, if there is formatting, we want to use the opening `{` and the
// next character, which will the `:` indicating the start of formatting. If there is
// not any formatting, we want to underline the entire span.
cx.arg_spans.get(self.cur_piece).map_or((None, None), |arg_span| {
if self.has_formatting {
(
Some(arg_span.with_lo(arg_span.lo() + BytePos(1)).shrink_to_lo()),
Some(arg_span.with_hi(arg_span.lo() + BytePos(2))),
)
} else {
let replace_start = arg_span.lo() + BytePos(1);
let replace_end = arg_span.hi() - BytePos(1);
let to_replace = arg_span.with_lo(replace_start).with_hi(replace_end);
(Some(to_replace), Some(*arg_span))
}
})
} else {
(None, None)
}

None
}
}

Expand Down Expand Up @@ -117,10 +137,18 @@ impl PositionalNamedArgsLint {
cur_piece: usize,
inner_span_to_replace: Option<rustc_parse_format::InnerSpan>,
names: &FxHashMap<Symbol, (usize, Span)>,
has_formatting: bool,
) {
let start_of_named_args = total_args_length - names.len();
if current_positional_arg >= start_of_named_args {
self.maybe_push(format_argument_index, ty, cur_piece, inner_span_to_replace, names)
self.maybe_push(
format_argument_index,
ty,
cur_piece,
inner_span_to_replace,
names,
has_formatting,
)
}
}

Expand All @@ -134,6 +162,7 @@ impl PositionalNamedArgsLint {
cur_piece: usize,
inner_span_to_replace: Option<rustc_parse_format::InnerSpan>,
names: &FxHashMap<Symbol, (usize, Span)>,
has_formatting: bool,
) {
let named_arg = names
.iter()
Expand All @@ -156,6 +185,7 @@ impl PositionalNamedArgsLint {
inner_span_to_replace,
replacement,
positional_named_arg_span,
has_formatting,
});
}
}
Expand Down Expand Up @@ -414,6 +444,9 @@ impl<'a, 'b> Context<'a, 'b> {
PositionalNamedArgType::Precision,
);

let has_precision = arg.format.precision != Count::CountImplied;
let has_width = arg.format.width != Count::CountImplied;

// argument second, if it's an implicit positional parameter
// it's written second, so it should come after width/precision.
let pos = match arg.position {
Expand All @@ -426,6 +459,7 @@ impl<'a, 'b> Context<'a, 'b> {
self.curpiece,
Some(arg.position_span),
&self.names,
has_precision || has_width,
);

Exact(i)
Expand All @@ -439,6 +473,7 @@ impl<'a, 'b> Context<'a, 'b> {
self.curpiece,
None,
&self.names,
has_precision || has_width,
);
Exact(i)
}
Expand Down Expand Up @@ -530,6 +565,7 @@ impl<'a, 'b> Context<'a, 'b> {
self.curpiece,
*inner_span,
&self.names,
true,
);
self.verify_arg_type(Exact(i), Count);
}
Expand Down Expand Up @@ -1152,24 +1188,22 @@ pub fn expand_format_args_nl<'cx>(

fn create_lints_for_named_arguments_used_positionally(cx: &mut Context<'_, '_>) {
for named_arg in &cx.unused_names_lint.positional_named_args {
let arg_span = named_arg.get_span_to_replace(cx);
let (position_sp_to_replace, position_sp_for_msg) = named_arg.get_positional_arg_spans(cx);

let msg = format!("named argument `{}` is not used by name", named_arg.replacement);
let replacement = match named_arg.ty {
PositionalNamedArgType::Arg => named_arg.replacement.to_string(),
_ => named_arg.replacement.to_string() + "$",
};

cx.ecx.buffered_early_lint.push(BufferedEarlyLint {
span: MultiSpan::from_span(named_arg.positional_named_arg_span),
msg: msg.clone(),
node_id: ast::CRATE_NODE_ID,
lint_id: LintId::of(&NAMED_ARGUMENTS_USED_POSITIONALLY),
diagnostic: BuiltinLintDiagnostics::NamedArgumentUsedPositionally(
arg_span,
named_arg.positional_named_arg_span,
replacement,
),
diagnostic: BuiltinLintDiagnostics::NamedArgumentUsedPositionally {
position_sp_to_replace,
position_sp_for_msg,
named_arg_sp: named_arg.positional_named_arg_span,
named_arg_name: named_arg.replacement.to_string(),
is_formatting_arg: named_arg.ty != PositionalNamedArgType::Arg,
},
});
}
}
Expand Down
17 changes: 11 additions & 6 deletions compiler/rustc_lint/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -856,13 +856,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");
if let Some(positional_arg) = positional_arg {
let msg = format!("this formatting argument uses named argument `{}` by position", name);
db.span_label(positional_arg, msg);
BuiltinLintDiagnostics::NamedArgumentUsedPositionally{ position_sp_to_replace, position_sp_for_msg, named_arg_sp, named_arg_name, is_formatting_arg} => {
db.span_label(named_arg_sp, "this named argument is referred to by position in formatting string");
if let Some(positional_arg_for_msg) = position_sp_for_msg {
let msg = format!("this formatting argument uses named argument `{}` by position", named_arg_name);
db.span_label(positional_arg_for_msg, msg);
}

if let Some(positional_arg_to_replace) = position_sp_to_replace {
let name = if is_formatting_arg { named_arg_name + "$" } else { named_arg_name };

db.span_suggestion_verbose(
positional_arg,
positional_arg_to_replace,
"use the named argument by name to avoid ambiguity",
name,
Applicability::MaybeIncorrect,
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_lint_defs/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,19 @@ pub enum BuiltinLintDiagnostics {
/// If true, the lifetime will be fully elided.
use_span: Option<(Span, bool)>,
},
NamedArgumentUsedPositionally(Option<Span>, Span, String),
NamedArgumentUsedPositionally {
/// Span where the named argument is used by position and will be replaced with the named
/// argument name
position_sp_to_replace: Option<Span>,
/// Span where the named argument is used by position and is used for lint messages
position_sp_for_msg: Option<Span>,
/// Span where the named argument's name is (so we know where to put the warning message)
named_arg_sp: Span,
/// String containing the named arguments name
named_arg_name: String,
/// Indicates if the named argument is used as a width/precision for formatting
is_formatting_arg: bool,
},
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
20 changes: 2 additions & 18 deletions src/bootstrap/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,20 +624,6 @@ impl Build {
/// If any submodule has been initialized already, sync it unconditionally.
/// This avoids contributors checking in a submodule change by accident.
pub fn maybe_update_submodules(&self) {
// WARNING: keep this in sync with the submodules hard-coded in bootstrap.py
let mut bootstrap_submodules: Vec<&str> = vec![
"src/tools/rust-installer",
"src/tools/cargo",
"src/tools/rls",
"src/tools/miri",
"library/backtrace",
"library/stdarch",
];
// As in bootstrap.py, we include `rust-analyzer` if `build.vendor` was set in
// `config.toml`.
if self.config.vendor {
bootstrap_submodules.push("src/tools/rust-analyzer");
}
// Avoid running git when there isn't a git checkout.
if !self.config.submodules(&self.rust_info) {
return;
Expand All @@ -653,10 +639,8 @@ impl Build {
// Look for `submodule.$name.path = $path`
// Sample output: `submodule.src/rust-installer.path src/tools/rust-installer`
let submodule = Path::new(line.splitn(2, ' ').nth(1).unwrap());
// avoid updating submodules twice
if !bootstrap_submodules.iter().any(|&p| Path::new(p) == submodule)
&& channel::GitInfo::new(false, submodule).is_git()
{
// Don't update the submodule unless it's already been cloned.
if channel::GitInfo::new(false, submodule).is_git() {
self.update_submodule(submodule);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/doc/rustc/src/command-line-arguments.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ but it is not guaranteed. If you need whole archive semantics use `+whole-archiv
This modifier is only compatible with the `static` linking kind.
Using any other kind will result in a compiler error.

When building a rlib or staticlib `+bundle` means that all object files from the native static
library will be added to the rlib or staticlib archive, and then used from it during linking of
the final binary.
When building a rlib or staticlib `+bundle` means that the native static library
will be packed into the rlib or staticlib archive, and then retrieved from there
during linking of the final binary.

When building a rlib `-bundle` means that the native static library is registered as a dependency
of that rlib "by name", and object files from it are included only during linking of the final
Expand Down
36 changes: 18 additions & 18 deletions src/test/ui/macros/issue-98466.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ warning: named argument `_x` is not used by name
--> $DIR/issue-98466.rs:7:26
|
LL | println!("_x is {}", _x = 5);
| - ^^ this named argument is only referred to by position in formatting string
| |
| this formatting argument uses named argument `_x` by position
| -- ^^ this named argument is referred to by position in formatting string
| |
| this formatting argument uses named argument `_x` by position
|
= note: `#[warn(named_arguments_used_positionally)]` on by default
help: use the named argument by name to avoid ambiguity
Expand All @@ -16,9 +16,9 @@ warning: named argument `y` is not used by name
--> $DIR/issue-98466.rs:10:26
|
LL | println!("_x is {}", y = _x);
| - ^ this named argument is only referred to by position in formatting string
| |
| this formatting argument uses named argument `y` by position
| -- ^ this named argument is referred to by position in formatting string
| |
| this formatting argument uses named argument `y` by position
|
help: use the named argument by name to avoid ambiguity
|
Expand All @@ -29,9 +29,9 @@ warning: named argument `y` is not used by name
--> $DIR/issue-98466.rs:13:83
|
LL | println!("first positional arg {}, second positional arg {}, _x is {}", 1, 2, y = _x);
| - ^ this named argument is only referred to by position in formatting string
| |
| this formatting argument uses named argument `y` by position
| -- ^ this named argument is referred to by position in formatting string
| |
| this formatting argument uses named argument `y` by position
|
help: use the named argument by name to avoid ambiguity
|
Expand All @@ -42,9 +42,9 @@ warning: named argument `_x` is not used by name
--> $DIR/issue-98466.rs:19:34
|
LL | let _f = format!("_x is {}", _x = 5);
| - ^^ this named argument is only referred to by position in formatting string
| |
| this formatting argument uses named argument `_x` by position
| -- ^^ this named argument is referred to by position in formatting string
| |
| this formatting argument uses named argument `_x` by position
|
help: use the named argument by name to avoid ambiguity
|
Expand All @@ -55,9 +55,9 @@ warning: named argument `y` is not used by name
--> $DIR/issue-98466.rs:22:34
|
LL | let _f = format!("_x is {}", y = _x);
| - ^ this named argument is only referred to by position in formatting string
| |
| this formatting argument uses named argument `y` by position
| -- ^ this named argument is referred to by position in formatting string
| |
| this formatting argument uses named argument `y` by position
|
help: use the named argument by name to avoid ambiguity
|
Expand All @@ -68,9 +68,9 @@ warning: named argument `y` is not used by name
--> $DIR/issue-98466.rs:25:91
|
LL | let _f = format!("first positional arg {}, second positional arg {}, _x is {}", 1, 2, y = _x);
| - ^ this named argument is only referred to by position in formatting string
| |
| this formatting argument uses named argument `y` by position
| -- ^ this named argument is referred to by position in formatting string
| |
| this formatting argument uses named argument `y` by position
|
help: use the named argument by name to avoid ambiguity
|
Expand Down
Loading

0 comments on commit e141246

Please sign in to comment.