Raw string literal escape depth is technically limited

[varkor]

This is not entirely serious, because I can't imagine this plausibly happening in real-world code (generated or otherwise), but in Nightly (Stable has a higher threshold):

fn main() {
    r#[... repeat 2^16 times ...]#""#[... repeat 2^16 times ...]#;
}

fails to parse with:

error: expected one of `.`, `;`, `?`, `}`, or an operator, found `#`
 --> src/main.rs:3:65544
  |
3 |     r######...
  | 

This limit is not mentioned in the reference.

I think we should document the maximum number of #s. We could also reduce the number permitted: 255 should be enough for anyone (#49993 (comment)).

Alternatively, we could ignore it as being a non-problem. Feel free just to close the issue 😄

[varkor]

On the other hand, this should make for a fairly straightforward beginner's contribution to the compiler: adding a specific error message for too many #s (and possibly reducing the maximum to 255).
(And ideally accompanying the change with a corresponding update to the reference.)

If anyone wants to do this to get started, I'm happy to mentor it (assuming it hasn't been closed 😄).

[steveklabnik]

@rust-lang/lang do you want to make some kind of guarantee here or just leave it open for implementations ?

[dcao]

This seems like a nice first issue for a beginner like me to contribute to, so I'd be happy to tackle this one (w/ some mentoring help)!

[varkor]

@CMDD: Great!

There are two places you'll need to modify in the parser for a custom error message:

rust/src/libsyntax/parse/lexer/mod.rs

Lines 1439 to 1446 in 52ed3d8

	'r' => {
	let start_bpos = self.pos;
	self.bump();
	let mut hash_count: u16 = 0;
	while self.ch_is('#') {
	self.bump();
	hash_count += 1;
	}

(Which parses raw string literals.)
And:

rust/src/libsyntax/parse/lexer/mod.rs

Lines 1669 to 1676 in 52ed3d8

	fn scan_raw_byte_string(&mut self) -> token::Lit {
	let start_bpos = self.pos;
	self.bump();
	let mut hash_count = 0;
	while self.ch_is('#') {
	self.bump();
	hash_count += 1;
	}

(Which parses raw byte string literals.)

For both, hash_count can be u8, and then you'll want to check the hash_count does not exceed the maximum value. If it does, you want to raise an error (fatal_span_ should work — there are examples in the same file) mentioning the limit.

Then you can lower the hash count field in the tokens themselves:

rust/src/libsyntax/parse/token.rs

Lines 76 to 78 in 52ed3d8

	StrRaw(ast::Name, u16), /* raw str delimited by n hash symbols */
	ByteStr(ast::Name),
	ByteStrRaw(ast::Name, u16), /* raw byte str delimited by n hash symbols */

You might need to update some other variables for this to type check (./x.py check is helpful here, and 4d34bfd is a similar commit).

Finally, make a new UI test (in src/test/ui — there are lots of examples there) testing the new error. Let me know if you need any more pointers!

[eddyb]

I don't understand why those use u16, when ast::Name aka Symbol is an u32 - unless the u16 gets reordered first?

[varkor]

@eddyb: I'm not sure about u32 vs u16, but it was recently changed from usize to u16 by this PR for space efficiency.

[eddyb]

I think #49993 (comment) confirms my suspicions. I find it odd though, that @nnethercote relied on field reordering instead of putting the u16 field first (although that would break more code).

[nnethercote]

@varkor: how did you find this -- from code inspection, or fuzzing, or something else?

@eddyb: why is it odd that I relied on field ordering? Is it not reliable?

[eddyb]

@nnethercote It was just not immediately obvious why u32 "followed" by u16 was useful size-wise, nor is there a comment about the packing being done, in the source.
Also, we have stricter conditions (that I can never remember) for enum field reordering, so if you want to enforce a specific layout, I'd suggest #[repr(u16)] (or u8) and placing the u16s first.

@varkor Also note that there are other limitations in rustc, although most of them are u32 aka 4Gi.

[varkor]

@nnethercote: I think I saw your PR and noticed that the limit was (slightly) more reachable.

@eddyb: yeah, that's not surprising. I decided to report as in the vein of #40161 (in which the examples are also often unrealistic), but it's probably sensible just to ignore these in general.

[steveklabnik]

Removing T-doc; this is basically an "implementation defined" kind of thing, and if we were to guarantee any length here, that would go on the reference. Leaving the issue open because it's tracking the diagnostic aspect.