serde
serde copied to clipboard
serde-rs
Create an extended version of syn::Meta that accepts paths and expressions in value position.
As discussed in #368 and #1490, there is a desire for supporting a larger set of syntaxes in attributes, including the ability to pass paths and expressions as values.
Right now, attribute parsing uses syn::Meta, but this only allows for literals in value position. This means that any other desired syntax must be wrapped in string literals (e.g. #[serde(default = "defaults::one")]). The long-term goal is to eventually replace such string literals with paths, and to be able to support other syntax elements in value position, such as expressions, for new kinds of options like default_value.
The approach that I'm taking is to duplicate and extend syn::Meta. I haven't bothered with changing the names or the basic structure as it seems to be otherwise sufficient, and this allows us to use it as a drop-in replacement with minimal changes. The main difference is that lit: Lit in MetaNameValue is replaced with lit: MetaValue, where MetaValue is an enum that allows types other than Lit to be parsed in its place.
Where I'm at now: I've replaced all uses of syn::Meta with the extended version, and it now supports path parsing. The only test failures are UI mismatches due to the different high-level parsing logic and also due to a new message "expected path or literal" in value position.
Adding support for expressions isn't as straightforward. ~~First, syn::Expr will require syn's "full" feature. I don't know what impact this will have on compile times, but it's worth considering whether that's acceptable.~~ (EDIT: I was mistaken, it is available with "derive", which is in our current feature set.) Second, there is ambiguity between paths, literals and expressions, as paths and literals are also valid expressions (e.g. std::mem::replace being an expression returning a function pointer). What I could do is completely replace MetaValue with Expr; though it might not be semantically correct to parse paths as expressions in all cases (e.g. #[serde(with = some::module)], it is still possible to match against the parsed expression and extract the path.
I'm having second thoughts about exactly specifying the type of values in MetaNameValue. It might make more sense to simply store it as a TokenStream and have a method like fn parse_value::<T: Parse>(&self) -> Result<T>. This is simultaneously the most flexible and most specific option, as it allows the value to be any kind of syntax element, and that type can be determined by each option in its own parsing code. We would be able to parse many other things like where clauses, types, and lifetimes.
This will also result in better diagnostic error messages for the end-user. If the type were specified within the structure of MetaNameValue, then it would also have to be parsed that early, and the error message would have to be as generic as possible, as nothing is known about what specific type is expected until the MetaNameValue is passed to an option for further parsing.
This pattern already exists in other places (e.g. syn::Attribute::parse_args), so I think it's worth trying out.
I had some issues with ambiguity when parsing values as TokenStreams, as they will parse every remaining token, including the punctuating commas. I'm looking into ways to resolve that, but I'm not very familiar with the proc macro ecosystem; feel free to add suggestions if you have any! My current plan is to make a custom parsing function for Punctuated that splits the token stream before parsing the individual nodes.
Example:
error: expected serde tag attribute to be a string: `tag = "..."`
--> test_suite/tests/test_gen.rs:604:19
|
604 | #[serde(tag = "t", content = "c")]
| ^^^^^^^^^^^^^^^^^^
I've been working on the next step, modifying the parsing frontend to accept inline syntax, you can see progress on my branch inline_values.
Something I ran into is an ambiguity with regard to where clauses. Since they use commas as punctuation, you can't write something like #[serde(bound = E: Clone, F: Clone)]. They would have to be written as separate invocations of the bound option, The other possibility I'm considering is allowing the where clause to be delimited with parentheses, so it would be written as #[serde(bound = (E: Clone, F: Clone))], but this definitely needs some discussion.
@dtolnay any feedback? There are a few UI failures but they don't seem to be any harder to understand, though a bit more verbose.
Hi @agausmann, I am still interested in this change but I haven't gotten a chance to take a look yet. It is still definitely on the list (https://triage.rust-lang.org/notifications?user=dtolnay).
I did a brief review, looks good to me. But I don't know the internals of serde that well to forsee any future problems.
