Open hanna-kruppe opened 2 months ago
Note that anything we do here would be a breaking change.
For myself, I feel like feature flags are a crude tool to work with and try as much as possible to reduce their use. From what you've described, I'm not sure there is enough benefit for this to outweigh that cost.
If we do anything with this, know of any good documentation on the difference between parsing
and full
with syn? The docs don't make it too clear what all is lost or what workarounds are available. I'm loath to use the
serde` approach of having attribute values be strings.
I haven't found much documentation on the difference when I looked into this, but there are clues. The doc comment for syn::Expr says "most of the variants are not available unless “full” is enabled" and each variant just wraps an ExprWhatever
struct that is annotated (on docs.rs) with what features it requires. Grouped by features and stripped of the common Expr prefix:
Looking at this list, ExprRange and possibly ExprArray ([a, b, c]
) seem to be the only "full"-exclusive ones that are likely to be commonly useful for clap (but I don't know the full API surface area by heart). On the other hand I'm surprised how many things are already enabled by "derive" alone (struct literals, method calls, macros, etc.). It seems that quite a few expressions kinds were moved out of full and into derive in the past year (e.g., struct literals in https://github.com/dtolnay/syn/commit/3b947f368a1d946b05bb36d10645fd41105e3718). I wonder why the line was drawn exactly there, and why this has changed over time. If syn can already parse that many different expressions without "full", perhaps the missing expression variants that clap needs could also be added with little cost?
Tuple
is another likely case.
Closure
is questionable. It can be convenient but anything larger than a single value should likely be a separate function.
If someone is willing to see if syn
would move these to derive
, I would be open to exploring this change during 5.0.
Please complete the following tasks
Clap Version
4.5.15
Describe your use case
I have a workspace where I care a little too much about build times (e.g., for iterating on cargo profile settings / RUSTFLAGS) and use a handful of syn-based custom derives, including clap's. Unlike most other derives I've seen (and unlike all others I'm using in this workspace), clap_derive enables syn's "full" feature that enables parsing arbitrary expressions, items, etc. instead of just the parts that are usually needed for derives. This has negative effects for compilation time (cc #2037):
cargo test
, out of ca. 10s total) because there happens to be enough independent work for the number of CPUs I have and another critical path in the schedule. It matters more in smaller workspaces (or with more CPUs, presumably), e.g., for the git-derive example by itself, disabling syn/full reduces clean dev builds from 5s to 4.3s on my machine.Describe the solution you'd like
Ideally, the syn/full feature could just be removed completely (cc #4784). clap_derive does build when the feature is removed (some care is needed while testing this because several dev-dependencies in clap's workspace enable it anyway), but some uses of the derives break because syn can't parse some kinds of expressions without the "full" feature. I don't have a full list of what's affected, but at least it causes an error for
arg(num_args = n..=m)
as used e.g., in the git-derive example. From my understanding of how clap_derive works, I think it's always possible to work around this by extracting those expressions into separate constants, but that's still annoying and a breaking change.On the other hand, many simpler expressions (literals, identifiers, paths, binary operators, taking references, etc.) don't seem to be affected. Case in point, my aforementioned workspace works just fine if test a patched version of clap_derive minus syn/full via
[patch.crates-io]
. So instead I propose a new feature, enabled by default, flag that controls whether syn/full is enabled. Those who care to fiddle around with feature flags and find that disabling this one doesn't break their project can do so, while everyone else is unaffected.Alternatives, if applicable
In addition to or instead of a new feature flag, the breaking change of not enabling syn/full (by default or ever) could be rolled into clap v5. But I don't seriously want to propose this without fully understanding the consequences. Are there any clap features that would become annoying or even impossible to use via the derive API without syn/full? For example, is there an ergonomic alternative for
num_args = n..=m
?Additional Context
No response