Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Improve heuristics whether format_args string is a source literal" #107041

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 2 additions & 35 deletions compiler/rustc_parse_format/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ pub use Flag::*;
pub use Piece::*;
pub use Position::*;

use rustc_lexer::unescape;
use std::iter;
use std::str;
use std::string;
Expand Down Expand Up @@ -314,11 +313,12 @@ impl<'a> Parser<'a> {
append_newline: bool,
mode: ParseMode,
) -> Parser<'a> {
let input_string_kind = find_width_map_from_snippet(s, snippet, style);
let input_string_kind = find_width_map_from_snippet(snippet, style);
let (width_map, is_literal) = match input_string_kind {
InputStringKind::Literal { width_mappings } => (width_mappings, true),
InputStringKind::NotALiteral => (Vec::new(), false),
};

Parser {
mode,
input: s,
Expand Down Expand Up @@ -856,7 +856,6 @@ impl<'a> Parser<'a> {
/// written code (code snippet) and the `InternedString` that gets processed in the `Parser`
/// in order to properly synthesise the intra-string `Span`s for error diagnostics.
fn find_width_map_from_snippet(
input: &str,
snippet: Option<string::String>,
str_style: Option<usize>,
) -> InputStringKind {
Expand All @@ -869,27 +868,8 @@ fn find_width_map_from_snippet(
return InputStringKind::Literal { width_mappings: Vec::new() };
}

// Strip quotes.
let snippet = &snippet[1..snippet.len() - 1];

// Macros like `println` add a newline at the end. That technically doens't make them "literals" anymore, but it's fine
// since we will never need to point our spans there, so we lie about it here by ignoring it.
// Since there might actually be newlines in the source code, we need to normalize away all trailing newlines.
// If we only trimmed it off the input, `format!("\n")` would cause a mismatch as here we they actually match up.
// Alternatively, we could just count the trailing newlines and only trim one from the input if they don't match up.
let input_no_nl = input.trim_end_matches('\n');
let Ok(unescaped) = unescape_string(snippet) else {
return InputStringKind::NotALiteral;
};

let unescaped_no_nl = unescaped.trim_end_matches('\n');

if unescaped_no_nl != input_no_nl {
// The source string that we're pointing at isn't our input, so spans pointing at it will be incorrect.
// This can for example happen with proc macros that respan generated literals.
return InputStringKind::NotALiteral;
}

let mut s = snippet.char_indices();
let mut width_mappings = vec![];
while let Some((pos, c)) = s.next() {
Expand Down Expand Up @@ -972,19 +952,6 @@ fn find_width_map_from_snippet(
InputStringKind::Literal { width_mappings }
}

fn unescape_string(string: &str) -> Result<string::String, unescape::EscapeError> {
let mut buf = string::String::new();
let mut error = Ok(());
unescape::unescape_literal(string, unescape::Mode::Str, &mut |_, unescaped_char| {
match unescaped_char {
Ok(c) => buf.push(c),
Err(err) => error = Err(err),
}
});

error.map(|_| buf)
}

// Assert a reasonable size for `Piece`
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Piece<'_>, 16);
Expand Down
12 changes: 12 additions & 0 deletions tests/ui/fmt/auxiliary/format-string-proc-macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub fn err_with_input_span(input: TokenStream) -> TokenStream {
TokenStream::from(TokenTree::Literal(lit))
}


#[proc_macro]
pub fn respan_to_invalid_format_literal(input: TokenStream) -> TokenStream {
let mut s = Literal::string("{");
Expand All @@ -38,3 +39,14 @@ pub fn respan_to_invalid_format_literal(input: TokenStream) -> TokenStream {
TokenTree::from(Group::new(Delimiter::Parenthesis, TokenTree::from(s).into())),
])
}

#[proc_macro]
pub fn capture_a_with_prepended_space_preserve_span(input: TokenStream) -> TokenStream {
let mut s = Literal::string(" {a}");
s.set_span(input.into_iter().next().unwrap().span());
TokenStream::from_iter([
TokenTree::from(Ident::new("format", Span::call_site())),
TokenTree::from(Punct::new('!', Spacing::Alone)),
TokenTree::from(Group::new(Delimiter::Parenthesis, TokenTree::from(s).into())),
])
}
9 changes: 9 additions & 0 deletions tests/ui/fmt/indoc-issue-106408.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// aux-build:format-string-proc-macro.rs
// check-pass

extern crate format_string_proc_macro;

fn main() {
let a = 0;
format_string_proc_macro::capture_a_with_prepended_space_preserve_span!("{a}");
}
9 changes: 7 additions & 2 deletions tests/ui/fmt/respanned-literal-issue-106191.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
// aux-build:format-string-proc-macro.rs
// check-fail
// known-bug: #106191
// unset-rustc-env:RUST_BACKTRACE
// had to be reverted
// error-pattern:internal compiler error
// failure-status:101
// dont-check-compiler-stderr

extern crate format_string_proc_macro;

fn main() {
format_string_proc_macro::respan_to_invalid_format_literal!("¡");
//~^ ERROR invalid format string: expected `'}'` but string was terminated
format_args!(r#concat!("¡ {"));
//~^ ERROR invalid format string: expected `'}'` but string was terminated
}
21 changes: 2 additions & 19 deletions tests/ui/fmt/respanned-literal-issue-106191.stderr
Original file line number Diff line number Diff line change
@@ -1,19 +1,2 @@
error: invalid format string: expected `'}'` but string was terminated
--> $DIR/respanned-literal-issue-106191.rs:6:65
|
LL | format_string_proc_macro::respan_to_invalid_format_literal!("¡");
| ^^^ expected `'}'` in format string
|
= note: if you intended to print `{`, you can escape it using `{{`

error: invalid format string: expected `'}'` but string was terminated
--> $DIR/respanned-literal-issue-106191.rs:8:18
|
LL | format_args!(r#concat!("¡ {"));
| ^^^^^^^^^^^^^^^^^^^^^^^ expected `'}'` in format string
|
= note: if you intended to print `{`, you can escape it using `{{`
= note: this error originates in the macro `concat` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors

query stack during panic:
end of query stack