Fix span of byte-escaped left format args brace #102214
Conversation
rustbot
added
the
T-compiler
|
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| @@ -224,7 +224,7 @@ impl<'a> Iterator for Parser<'a> { | |||
| '{' => { | |||
| let curr_last_brace = self.last_opening_brace; | |||
| let byte_pos = self.to_span_index(pos); | |||
| let lbrace_end = InnerOffset(byte_pos.0 + 1); | |||
| let lbrace_end = self.to_span_index(pos + 1); | |||
There was a problem hiding this comment.
There is a manual creation of an InnerOffset just below (github won't let me comment on it). Could it trigger a similar bug? Should it be changed in a similar way?
There was a problem hiding this comment.
That's a good point. Originally I had assumed not since I wasn't able to find any breaking examples, but I just found that following:
println!("{\x7D");gives the following error:
error: 1 positional argument in format string, but no arguments were given
--> src/main.rs:2:15
|
2 | println!("{\x7D");
| ^^
I can go ahead and apply the fix to that line as well, and add some more tests.
There was a problem hiding this comment.
Yes, please, with the test if possible. That's a good opportunity to fix several identical bugs at once. I'll approve once this is done.
There was a problem hiding this comment.
Fixed in efc6df2 (edit: this resulted in a regression, see follow-up comment).
cassaundra
force-pushed
the
fix-format-args-span
branch
from
ebc0a52
to
efc6df2
Compare
|
Looking more into this issue, it seems like the way that format strings handle skipped characters erases some necessary information about the source string. Consider these two statements: println!("{\x7D"};
println!("{}\
");In both cases, the resultant interned string looks like I'm thinking that it would be best to refactor this skipping mechanism to instead deal with a sequence of character widths (essentially squashing together some of the skips). It would make this PR larger than it initially was, but that's fine with me. Thoughts? |
This comment has been minimized.
This comment has been minimized.
|
@cassaundra a follow-up PR sounds good. If you don't mind, let's go back to the previous version of the patch, so we can get it merged in the mean time. |
cassaundra
force-pushed
the
fix-format-args-span
branch
from
efc6df2
to
e5096d4
Compare
|
Done. |
cassaundra
changed the title
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…=cjgillot Fix span of byte-escaped left format args brace Fix rust-lang#102057 (see issue for example). Previously, the use of escaped left braces (`\x7B`) in format args resulted in an incorrectly offset span. This patch fixes that by considering any escaped characters within the string instead of using a constant offset.
Rollup of 7 pull requests Successful merges: - rust-lang#102214 (Fix span of byte-escaped left format args brace) - rust-lang#102426 (Don't export `__wasm_init_memory` on WebAssembly.) - rust-lang#102437 (rustdoc: cut margin-top from first header in docblock) - rust-lang#102442 (rustdoc: remove bad CSS font-weight on `.impl`, `.method`, etc) - rust-lang#102447 (rustdoc: add method spacing to trait methods) - rust-lang#102468 (tidy: make rustc dependency error less confusing) - rust-lang#102476 (Split out the error reporting logic into a separate function) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…cjgillot Fix incorrect span when using byte-escaped rbrace Fix rust-lang#103826, a format args span issue introduced in rust-lang#102214. The current solution for tracking skipped characters made it so that certain situations were ambiguous enough that the original span couldn't be worked out later. This PR improves on the original solution by keeping track of groups of skipped characters using a map, and fixes the previous bug. See an example of this ambiguity in the [previous PR's discussion](rust-lang#102214 (comment)).
Fix incorrect span when using byte-escaped rbrace Fix #103826, a format args span issue introduced in #102214. The current solution for tracking skipped characters made it so that certain situations were ambiguous enough that the original span couldn't be worked out later. This PR improves on the original solution by keeping track of groups of skipped characters using a map, and fixes the previous bug. See an example of this ambiguity in the [previous PR's discussion](rust-lang/rust#102214 (comment)).
Fix incorrect span when using byte-escaped rbrace Fix #103826, a format args span issue introduced in #102214. The current solution for tracking skipped characters made it so that certain situations were ambiguous enough that the original span couldn't be worked out later. This PR improves on the original solution by keeping track of groups of skipped characters using a map, and fixes the previous bug. See an example of this ambiguity in the [previous PR's discussion](rust-lang/rust#102214 (comment)).
Fix incorrect span when using byte-escaped rbrace Fix #103826, a format args span issue introduced in #102214. The current solution for tracking skipped characters made it so that certain situations were ambiguous enough that the original span couldn't be worked out later. This PR improves on the original solution by keeping track of groups of skipped characters using a map, and fixes the previous bug. See an example of this ambiguity in the [previous PR's discussion](rust-lang/rust#102214 (comment)).
Fix #102057 (see issue for example).
Previously, the use of escaped left braces (
\x7B) in format args resulted in an incorrectly offset span. This patch fixes that by considering any escaped characters within the string instead of using a constant offset.