Support arbitrary token streams in list attributes.

[withoutboats]

With this feature, given an attribute #[foo(..)], the .. can be an
arbitrary token stream.

[rust-highfive]

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

[petrochenkov]

r? @jseyfried

[withoutboats]

Added a test which doesn't pass, still figuring out what's wrong here.

[nikomatsakis]

@withoutboats how goes -- any progress here?

[nikomatsakis]

@withoutboats would you like someone to take a closer look? (I've not looked closely at this PR yet)

[withoutboats]

@nikomatsakis Would be good!

I thought adding a fallback to MetaItem for cases where it doesn't conform to "nested meta item" syntax would be sufficient (while still supporting the use cases where we evaluate nested meta items in libsyntax today), but its still not parsing correctly and I got lost in the parser trying to figure out where the error was arising from.

[nikomatsakis]

cc @petrochenkov -- any thoughts on this?

(I'll try to dig into it later on, ran out of time today.)

[petrochenkov]

@nikomatsakis

any thoughts on this?

I didn't follow development of token streams and procedural macros 2.0, so I'm not aware of any possible subtleties this change can result in, but in general, if custom attributes are procedural macros, then it seems they should indeed accept arbitrary token trees.

Possible issues:

• Arbitrary token trees in attributes that are not procedural macros, attribute validation.
• MetaItemKind::TokenStream seems to subsume other variants except for NameValue maybe, how are they disambiguated during parsing?

Well, at least it's feature gated, so it's won't be a big problem if something goes unexpectedly wrong.

[withoutboats]

@petrochenkov The way I intended the code to work (though it seems like it isn't) is to fallback to TokenStream if it can't parse the attribute as a NestedMetaItem. That way anything that conforms to our existing built in attributes will continue to be parsed as a NestedMetaItem, avoiding rewriting all the code that walks that variant to process built in attributes.

[withoutboats]

Also I'd like to stabilize this feature this cycle so that the custom attributes registered by derive can use this syntax.

[jseyfried]

Sorry for delay getting to this; I'll investigate test failure this weekend.

[carols10cents]

Triage ping -- looks like this is in @nikomatsakis and @jseyfried 's courts to investigate?

[nikomatsakis]

OK, @withoutboats, I'm building a copy of this branch locally and will try to investigate.

[nikomatsakis]

@withoutboats I took a peek. I believe the problem is that the parser "eagerly commits" to a parse error sometimes. Fixing this will require some mild refactoring. Let me show you what I mean.

As part of parsing a metadata item, we invoke parse_seq_to_end. This invokes parse_seq_to_before_end, which invokes parse_seq_to_before_tokens. But something curious happens along the way -- parse_seq_to_end returns a PResult<>, where the Err case is an unemitted error. But if you look closely you will see it is always returning Ok:

rust/src/libsyntax/parse/parser.rs

Lines 980 to 990 in cbf5d39

	pub fn parse_seq_to_end<T, F>(&mut self,
	ket: &token::Token,
	sep: SeqSep,
	f: F)
	-> PResult<'a, Vec<T>> where
	F: FnMut(&mut Parser<'a>) -> PResult<'a, T>,
	{
	let val = self.parse_seq_to_before_end(ket, sep, f);
	self.bump();
	Ok(val)
	}

parse_seq_to_before_end, in contrast, does not return a Vec:

rust/src/libsyntax/parse/parser.rs

Lines 995 to 1003 in cbf5d39

	pub fn parse_seq_to_before_end<T, F>(&mut self,
	ket: &token::Token,
	sep: SeqSep,
	f: F)
	-> Vec<T>
	where F: FnMut(&mut Parser<'a>) -> PResult<'a, T>
	{
	self.parse_seq_to_before_tokens(&[ket], sep, TokenExpectType::Expect, f, |mut e| e.emit())
	}

In fact, it invokes parse_seq_to_before_tokens, which takes a closure that declares what to do in the case of error. In this case, it is invoking |mut e| e.emit(), which will emit the error out.

So basically this is the problem. We need to refactor parse_seq_to_before_tokens to return a PResult -- probably what we want to do is to modify the callback so that it can return a PResult<()>. However, I don't fully understand the interaction here with parser fallback -- that is, if an error occurs, and we want to keep productively parsing, there may be some interaction here. @nrc may know more on this particular topic.

[shepmaster]

Howdy from triage, @withoutboats! It's been over a week since we last heard from you here. Will you have time to work on the test failures soon?

[shepmaster]

Thanks for the pr @withoutboats ! Unfortunately, we haven't heard from you in a few weeks, so I'm going to close this to keep things tidy. Please reopen when you have a chance to address the remaining issues!