rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.73k stars 2.42k forks source link

Support multi-line inline tables #11500

Open poscat0x04 opened 1 year ago

poscat0x04 commented 1 year ago

Problem

Currently cargo won't parse a cargo.toml file that contains multi-line inline tables.

Proposed Solution

Change the toml parser to allow multi-line inline tables

Notes

No response

weihanglo commented 1 year ago

Note that “newlines and trailing commas in inline tables” is still unreleased.

https://github.com/toml-lang/toml/blob/913ff560d935e7fb2788b402abba567680f6e03c/CHANGELOG.md

epage commented 1 year ago

Yes, we currently support TOML 1.0.

epage commented 1 year ago

Upstream issue toml-rs/toml#397

A downside to the new TOML version is it could cause a 10% slow down in TOML parsing without a lot of optimization work as it adds support for unicode bare keys. Unless we implement a UTF-8 state machine in our parser (and deal with the risks associated with it), it'll require us to move from &[u8] parsing to &str parsing.

poscat0x04 commented 1 year ago

Ah ok. Didn't know that toml has a release cycle.

A downside to the new TOML version is it could cause a 10% slow down in TOML parsing without a lot of optimization work as it adds support for unicode bare keys. Unless we implement a UTF-8 state machine in our parser (and deal with the risks associated with it), it'll require us to move from &[u8] parsing to &str parsing.

Parsing toml shouldn't be a performance bottleneck, right? IMO a 10% slowdown is acceptable.

weihanglo commented 1 year ago

Parsing toml shouldn't be a performance bottleneck, right? IMO a 10% slowdown is acceptable.

Parse does contribute to total build time to some extent. Cargo parses Cargo.toml for each dependency, as well as configuration files and Cargo.lock. Back in the day we perceived a performance loss when switching from toml-rs to toml_edit. Anyhow, let's keep an eye on it :)

epage commented 1 year ago

The biggest concern for performance is parsing someone's dependency tree. We do it on every run and, for large programs, can involve parsing hundreds of toml files. Toml is designed for humans and isn't the most optimal format to parse already.

One potential area of improvement is if we track immutable manifests and cache those in a fast to parse format. The biggest gains for immutable manifests would be from the registry. A manifest for a given git dependency's SHA would also be immutable but I wouldn't expect there to be too many of those to be worth it.

pronebird commented 6 months ago

Can we please fix this, it's really surprising that this is even an issue for a text parser.

epage commented 6 months ago

TOML 1.1 spec is not released yet.