Skip to content

Commit

Permalink
Auto merge of rust-lang#99263 - compiler-errors:issue-99261, r=jyn514
Browse files Browse the repository at this point in the history
Fix ICE in `named_arguments_used_positionally` lint

Fixes rust-lang#99261
Fixes rust-lang#99289
Fixes rust-lang#99284
Fixes rust-lang#99273
Fixes rust-lang#99297
Fixes rust-lang#99271

This match pattern:

```
 FormatSpec { width: Count::CountIsName(s, _), .. }
| FormatSpec { precision: Count::CountIsName(s, _), .. }
```

does not account for when both `width` and `precision` are both `Count::CountIsName`, so split the check for these two fields into two separate `if let`.

Also, remove any future potential for ICEs by removing the index operator altogether.

---

It is still suspicious that this indexing was broken and caused the ICE, as opposed to just causing a spurious lint message.

cc `@PrestonFrom,` who may be familiar with this code because of implementing the lint this touches, perhaps you'd like to look into why named arguments in `FormatSpec.precision` seem to have indices that don't correspond to a span in `Context.arg_spans`?

Edit: Opened rust-lang#99265 to track a (related?) incorrect argument indexing issue.
  • Loading branch information
bors committed Jul 16, 2022
2 parents d695a49 + 2902b92 commit 5635158
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 19 deletions.
17 changes: 8 additions & 9 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +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, FormatSpec};
use rustc_parse_format::Count;
use std::borrow::Cow;
use std::collections::hash_map::Entry;

Expand Down Expand Up @@ -985,20 +985,19 @@ fn lint_named_arguments_used_positionally(
}
_ => {}
};
match a.format {
FormatSpec { width: Count::CountIsName(s, _), .. }
| FormatSpec { precision: Count::CountIsName(s, _), .. } => {
used_argument_names.insert(s);
}
_ => {}
};
if let Count::CountIsName(s, _) = a.format.width {
used_argument_names.insert(s);
}
if let Count::CountIsName(s, _) = a.format.precision {
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];
let arg_span = cx.arg_spans.get(index).copied();
cx.ecx.buffered_early_lint.push(BufferedEarlyLint {
span: MultiSpan::from_span(span),
msg: msg.clone(),
Expand Down
19 changes: 10 additions & 9 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,15 +858,16 @@ pub trait LintContext: Sized {
},
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,
);

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);
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.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ pub enum BuiltinLintDiagnostics {
/// If true, the lifetime will be fully elided.
use_span: Option<(Span, bool)>,
},
NamedArgumentUsedPositionally(Span, Span, String),
NamedArgumentUsedPositionally(Option<Span>, Span, String),
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/macros/issue-99261.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// check-pass

#![deny(named_arguments_used_positionally)]

fn main() {
let value: f64 = 314.15926;
let digits_before_decimal = 1;
let digits_after_decimal = 2;
let width = digits_before_decimal + 1 + digits_after_decimal;

println!(
"{value:0>width$.digits_after_decimal$}",
value = value,
width = width,
digits_after_decimal = digits_after_decimal,
);
}

0 comments on commit 5635158

Please sign in to comment.