Port cargo from toml-rs to toml_edit

[epage]

Benefits:

• A TOML 1.0 compliant parser
• Unblock future work
  • Have cargo init add the current crate to the workspace, rather
    than error
  • Add cargo-add (from cargo-edit) to cargo proper #5586: Upstream cargo-add

TODO

• Analyze performance and address regressions
• Identify and resolve incompatibiies
• Resolve remaining test failures, see
  https://github.com/ordian/toml_edit/labels/cargo
• Switch the code from Check --config for dotted keys only #10176 to only parse once (this PR is being merged first)

[rust-highfive]

r? @Eh2406

(rust-highfive has picked a reviewer for you, use r? to override)

[epage]

Oh, if I knew highfive looks at drafts, I might not have created this.

[gilescope]

Does toml_edit still have a way to do spanned so we can (one day) get Cargo.toml line numbers? I tried it with toml-rs but got stuck with 4 layers of serde... ( #9752 )

[epage]

Does toml_edit still have a way to do spanned so we can (one day) get Cargo.toml line numbers? I tried it with toml-rs but got stuck with 4 layers of serde... ( #9752 )

I saw references to spans in toml-rs but never saw what they were used for, so I assumed it was an implementation detail.

Mind creating an issue in the toml_edit repo with details of what you would find useful in dealing with this or what works or doesn't work with toml-rs's version?

[est31]

Spans are useful for creating nice errors that quote the actual toml, similar to how rustc is creating errors that quote your rust source code. Also, they are useful for IDEs so that not the entire file is underlined if there is an error. One step towards such errors is cargo creating span information. #7548

[jonhoo]

Some additional anecdotal data for why this might be worth pushing on. Here's cargo-llvm-lines run against a library that links against Cargo:

  Lines          Copies        Function name
  -----          ------        -------------
  448018 (100%)  10087 (100%)  (TOTAL)
   25227 (5.6%)      5 (0.0%)  <cargo::util::toml::_::<impl serde::de::Deserialize for cargo::util::toml::TomlProject>::deserialize::__Visitor as serde::de::Visitor>::visit_map
   12136 (2.7%)     34 (0.3%)  <toml::de::MapVisitor as serde::de::MapAccess>::next_value_seed
   11721 (2.6%)      6 (0.1%)  <cargo::util::toml::_::<impl serde::de::Deserialize for cargo::util::toml::TomlProfile>::deserialize::__Visitor as serde::de::Visitor>::visit_map
   10582 (2.4%)      5 (0.0%)  <cargo::util::toml::_::<impl serde::de::Deserialize for cargo::util::toml::TomlTarget>::deserialize::__Visitor as serde::de::Visitor>::visit_map
   10218 (2.3%)     59 (0.6%)  <toml::de::SpannedDeserializer<T> as serde::de::MapAccess>::next_value_seed
    9269 (2.1%)     12 (0.1%)  <toml::de::MapVisitor as serde::de::MapAccess>::next_key_seed
    9140 (2.0%)     25 (0.2%)  <toml::de::ValueDeserializer as serde::de::Deserializer>::deserialize_any
    7245 (1.6%)      2 (0.0%)  <cargo::util::toml::_::<impl serde::de::Deserialize for cargo::util::toml::TomlManifest>::deserialize::__Visitor as serde::de::Visitor>::visit_map
    6717 (1.5%)      3 (0.0%)  <cargo::util::toml::_::<impl serde::de::Deserialize for cargo::util::toml::DetailedTomlDependency<P>>::deserialize::__Visitor<P> as serde::de::Visitor>::visit_map
    6467 (1.4%)     19 (0.2%)  <toml::de::MapVisitor as serde::de::SeqAccess>::next_element_seed

It'd be interesting to see if the move to toml_edit could help with all that.

[epage]

Tests are now passing for me locally. While I still need to dig into performance, I'd appreciate feedback on what is acceptable or not for user-visible changes.

Good

• We now include a newline before [dependencies.bar] in published toml files
• We skip empty [dev-dependencies] in published toml files

Unsure

• toml_edit has a limited form of showing where in a line the error came from
• Errors for a line with just a key now report both a . and = as next possible character
• Error messages only reported unexpected character (Unexpected "b") and not token (Unexpected identifier)

Remaining not greats

• While value=zzz provides a nice error message (unexpected z, expected quoted string), but not value=[zzz] (Expected 'newline' or '#') because of challenges with parsing optional trailing commas (Error messages for value=[zzz] are not helpful toml-rs/toml#293)
• Unexpected \n shows as an actual newline (Rendering newline in error doesn't help user toml-rs/toml#259)
• Unexpected \0 shows as an invisible character (Nulls are invisible in errors (Unexpecred '\0') toml-rs/toml#294)
• Errors quote non-literal text (Errors quote non-literal text toml-rs/toml#295)
• Some suggested syntax is missing (Incomplete inline tables should say either key or } is needed toml-rs/toml#296)
• There are some differences in serializing strings (Rendering of strings doesn't match toml-rs toml-rs/toml#287)
• In some cases, cargo indents the toml_edit multi-line error messages and in some cases, it only indents the first line

[alexcrichton]

The changes to the tests here all look quite reasonable to me. While there could be small nits about one way or another in a few places overall I think all the substance is still there which is largely what matters.

[epage]

Performance looks to be a wash to around 5% slow down. Its hard to tell exactly because most of the change is within the margin of error on my laptop

• rustc 1.57
• Linux
• Plugged in
• Didn't close all apps but did stop music and things
• Runs varied for both master and this branch by about 5%

So onto my last run of this branch compared to my last master run

85f7bfd61ea4fee08ec68c468762e886b2aebec6          
resolve_ws/cargo        time:   [30.337 ms 30.411 ms 30.502 ms]                              
                        change: [-3.5217% -2.6600% -1.8773%] (p = 0.00 < 0.05)
                        Performance has improved.    
Found 5 outliers among 100 measurements (5.00%)                                                                                                    
  1 (1.00%) high mild                                                                                                                              
  4 (4.00%) high severe                                                                                                                            
Benchmarking resolve_ws/diem: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 25.3s, or reduce sample count to 10.
resolve_ws/diem         time:   [254.70 ms 255.38 ms 256.17 ms]                             
                        change: [-2.4359% -1.7454% -1.0574%] (p = 0.00 < 0.05)                  
                        Performance has improved.                                                                                                  
Found 2 outliers among 100 measurements (2.00%)                                                                                                    
  1 (1.00%) high mild                                                    
  1 (1.00%) high severe                                                  
resolve_ws/empty        time:   [247.70 us 249.18 us 250.78 us]                              
                        change: [+4.6698% +5.5148% +6.4177%] (p = 0.00 < 0.05)                                                                     
                        Performance has regressed.                                                                                                 
Found 5 outliers among 100 measurements (5.00%)                                                                                                    
  4 (4.00%) high mild                                                    
  1 (1.00%) high severe                                                  
Benchmarking resolve_ws/gecko-dev: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 20.9s, or reduce sample count to 20.
resolve_ws/gecko-dev    time:   [206.65 ms 207.81 ms 209.03 ms]                                   
                        change: [+4.7307% +5.5707% +6.4154%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild                                                    
Benchmarking resolve_ws/rust: Warming up for 3.0000 s                                                                                              
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 15.8s, or reduce sample count to 30.
resolve_ws/rust         time:   [154.89 ms 155.65 ms 156.43 ms]                             
                        change: [+3.9641% +4.8089% +5.6012%] (p = 0.00 < 0.05)
                        Performance has regressed.                       
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild                                                                                                                              
Benchmarking resolve_ws/servo: Warming up for 3.0000 s         
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 37.8s, or reduce sample count to 10.
resolve_ws/servo        time:   [383.39 ms 385.20 ms 387.04 ms]                              
                        change: [+2.3391% +3.0466% +3.8055%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
Benchmarking resolve_ws/substrate: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 39.8s, or reduce sample count to 10.
resolve_ws/substrate    time:   [392.95 ms 394.53 ms 396.16 ms]                                  
                        change: [+5.8655% +6.4134% +7.0062%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
Benchmarking resolve_ws/tikv: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 25.0s, or reduce sample count to 20.
resolve_ws/tikv         time:   [247.03 ms 248.35 ms 249.73 ms]                             
                        change: [+6.3043% +6.9944% +7.7274%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
resolve_ws/toml-rs      time:   [4.1845 ms 4.2097 ms 4.2380 ms]                                 
                        change: [+11.882% +12.879% +13.816%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe

85f7bfd61ea4fee08ec68c468762e886b2aebec6
feature_resolver/cargo  time:   [516.88 us 519.76 us 522.81 us]                                    
                        change: [+4.2772% +5.0601% +5.8259%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
feature_resolver/diem   time:   [6.4566 ms 6.5098 ms 6.5634 ms]                                   
                        change: [+2.9215% +3.8941% +4.9640%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild
feature_resolver/empty  time:   [2.9904 us 3.0063 us 3.0238 us]                                     
                        change: [+5.4619% +6.3317% +7.2605%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) high mild
  3 (3.00%) high severe
feature_resolver/gecko-dev                                                                                                                         
                        time:   [3.3601 ms 3.3777 ms 3.3957 ms]
                        change: [+4.4394% +5.3523% +6.2079%] (p = 0.00 < 0.05)                                               
                        Performance has regressed.                                                                                                 
Found 1 outliers among 100 measurements (1.00%)                                                                                                    
  1 (1.00%) high mild                                                    
feature_resolver/rust   time:   [3.7609 ms 3.7908 ms 3.8233 ms]                                    
                        change: [+4.2661% +5.2651% +6.2767%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)           
  3 (3.00%) high mild                                                                                                                              
  1 (1.00%) high severe                                                                                                                            
feature_resolver/servo  time:   [4.8117 ms 4.8450 ms 4.8810 ms]                                    
                        change: [+2.7391% +4.0271% +5.1739%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe                                                  
feature_resolver/substrate                                                                                                   
                        time:   [25.665 ms 25.833 ms 26.028 ms]                                                                                    
                        change: [+6.3875% +7.3589% +8.2665%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe                                                                                                                            
feature_resolver/tikv   time:   [7.3014 ms 7.3603 ms 7.4201 ms]                                  
                        change: [+4.6716% +5.6024% +6.5760%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
feature_resolver/toml-rs                                                                              
                        time:   [30.400 us 30.562 us 30.736 us]                                                                                    
                        change: [+4.9207% +5.5802% +6.1710%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

[epage]

@Eh2406 if we're comfortable with these performance numbers, then this is ready for review

(unsure if the change away from draft status pings people)

[bors]

📌 Commit 320c279 has been approved by ehuss

[bors]

⌛ Testing commit 320c279 with merge bb96b3a...

[bors]

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing bb96b3a to master...