Refactor #[wasm_bindgen] attribute parsing

[TedDriggs]

This commit moves to having a struct per syn::Item type that the macro operates on, then uses darling to handle validation of the attribute.

This implicitly fixes #2874, since darling automatically generates errors for unexpected properties. It also makes the macro code easier to follow by hiding the complexity associated with parsing out of sight in a well-tested crate. This also precludes a class of bug where someone might have attempted to read a property that was not accepted for that syntax item, since each syntax item now has its own options receiver.

darling operates on syn::Meta, which only allows a literal to the right side of = signs in attributes. This is a problem for wasm_bindgen, which does not use them.

To maintain API compatibility while using darling, this commit adds macro_support::meta::Meta<T>, a replica of syn::Meta where Lit is replaced by a type parameter. This may be useful enough beyond wasm_bindgen for it to move upstream to darling, which would mean this change would reduce the overall amount of code in macro-support.

[TedDriggs]

@alexcrichton it's no longer a breaking change, and the size is coming down! (And may come down further if I do end up moving the new meta::Meta out to be darling::ast::Meta)

(Asterisk on the breaking change piece: In #2874, an invalid meta-item is silently ignored, while this change will cause that to be a compile error. So this could be "breaking" in that sense, but no valid usages will break from this)

[alexcrichton]

Sorry but this is still a pretty big PR with a new crate that I've never personally used before so I don't have prior experience to help me with review. Additionally while the pain points mentioned here I think would be good to fix they don't seem like critical bugs and I'm not really in a position to review major changes to wasm-bindgen at the moment.

[TedDriggs]

Thanks for the reply. I don't know of a way to further shrink the PR, so I'll leave it as-is in case things change and time for review becomes available.

[daxpedda]

I actually like darling and used it a couple of times with great pleasure, but I'm generally against adding it to wasm-bindgen.

Generally speaking I would avoid adding any further proc-macro crates on top of syn in fundamental crates like wasm-bindgen.

But if other maintainers speak in favor of adding this, I would be happy to review it.

[TedDriggs]

@daxpedda that build-time impact is gross.

How did you make this chart? I currently don't know anything about reducing build times for Rust crates, but this is reason to start learning.

--

Is build-time impact your primary reason for opposition? I'm totally fine with this PR being closed out, but darling's long-term goal is to be "serde for proc-macro parsing", where its feature set is compelling enough - and its barriers to entry, including performance impact - are low enough that there are only rare cases where it shouldn't be a good solution.

Given that goal, any other concerns you have sound like they should be areas of exploration for darling's roadmap

[daxpedda]

How did you make this chart?

See the Cargo documentation on timings.

Is build-time impact your primary reason for opposition?

Yes, more reasoning to follow.

[..] only rare cases where it shouldn't be a good solution

I think core crates in general should have as little dependencies as possible in Rust (probably grossly off-topic to discuss why here).

So generally speaking, I think this is one of these rare cases. syn can't really be avoided, e.g. parsing types by hand is probably a terrible idea, but darling is avoidable.

The only way I would re-consider is if the crate has very little overhead and provides a significant (quite subjective) improvement over just using syn.

[TedDriggs]

I’d contend that good error handling for a project like this is very important - and is an example of the “undifferentiated heavy lifting” that makes sense to outsource to another crate. When I was learning Rust, I found the quality of the compiler errors to be hugely valuable, and I was disappointed by the comparative lack of quality in proc-macro errors.

darling is designed to provide a few soft guarantees:

1. Meta item order doesn’t matter by default; this was a source of major parsing complexity when darling started out, and you can still see that verbose code on display in darling_core
2. One run of the macro will return every error it can; a naive implementation using ? will only return one error at a time for macro invocation, which is a much worse dev experience
3. Errors will be spanned correctly- getting a macro error that points at the macro call site is very frustrating, especially on a large struct.

The crate also has some quality-of-life features that I haven’t seen in most hand-rolled macros; key among those is the “did you mean” suggestions when encountering an unexpected field.

I’m not sure if I’ll be able to get the build time down to a point where it’s good enough for a core crate such as this, but if I can I would contend that the benefits listed above are important for ease of adoption, particularly for people who might be trying Rust for the first time in a WASM context

[daxpedda]

I think a significant increase in error message quality, which isn't hard considering it's currently quite low in wasm-bindgen, would make darling definitely worth it, in my opinion at least.

So I would definitely welcome the improvement if darling manages to reduce the effect on build times.

[TedDriggs]

@daxpedda I've filed TedDriggs/darling#237 to investigate improvements here - if you have any experience with reading the time-passes data from rustc and are willing to give it a look, that'd be much appreciated because I'm not sure how to proceed.