internal: Support Pointee trait

[HKalbasi]

fix #13992

[HKalbasi]

This PR upgrades chalk which uses syn2. Is it a problem?

[Veykril]

I'd like to wait until we can move all our dependencies over to versions using syn 2 so we don't build syn 1 and syn 2

[lnicola]

That mean we won't be able to get any juicy chalk fixes until we switch, not that it's really active these days.

I have a draft PR for salsa at salsa-rs/salsa#442, but it still needs some work.

[Veykril]

fwiw, we can't really use upstream salsa as is. There are some commits on master that the latest release doesn't have. And those commits cause a considerable slow down in rust-analyzer.

So we'd need to branch a release off with those commits omitted (and ideally with the commit that updates parking_lot included since thats currently duped for us due to salsa as well)

[HKalbasi]

Now that we don't publish crates, isn't possible to use git dependency to our forks? Either for salsa to upgrade syn, or for chalk to downgrade syn.

[lnicola]

I see, you mean that slot change. I tried to run the completion benchmark again and got:

# master
running 1 test
workspace loading: 9.07s
initial: 1.24s
change: 48.98µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
   91ms - unqualified path completion
       21ms - CompletionContext::new
           10ms - parse_query @ FileId(45)
            3ms - CompletionContext::analyze (1 calls)
            5ms - CompletionContext::expand (1 calls)
            0   - Semantics::analyze_impl (1 calls)
            0   - crate_def_map:wait (1 calls)
       38ms - import_on_the_fly @ sel
           22ms - import_assets::search_for_imports
               22ms - import_assets::search_for
                   22ms - import_assets::path_applicable_imports
                       21ms - items_with_name @ Name: sel, crate: Exclude, assoc items: Some("hir"), limit: Some(40)
                           21ms - find_items
                               21ms - crate_symbols
                                    0   - crate_def_map:wait (1 calls)
                                   21ms - module_symbols (11 calls)
                                0   - query_external_importables (1 calls)
                                0   - symbol_index::Query::search (1 calls)
                        0   - find_path_prefixed (45 calls)
                        0   - get_name_definition (40 calls)
                    0   - Semantics::analyze_impl (1 calls)
                    0   - import_assets::scope_definitions (1 calls)
            0   - attrs_query (7 calls)
            0   - crate_def_map:wait (3 calls)
            0   - item::Builder::build (35 calls)
           15ms - render_resolution (35 calls)
       13ms - complete_expr_path
           13ms - CompletionContext::process_all_names
                0   - attrs_query (27 calls)
                0   - crate_def_map:wait (344 calls)
                0   - item::Builder::build (303 calls)
               12ms - render_resolution (303 calls)
            0   - item::Builder::build (13 calls)
       18ms - iterate_method_candidates
            0   - PerNs::filter_visibility (24 calls)
            0   - crate_def_map:wait (384 calls)
            0   - deref_by_trait (7 calls)
            0   - generic_params_query (209 calls)
            0   - item::Builder::build (134 calls)
           13ms - render_method (134 calls)
            0   - resolve_obligations_as_possible (17 calls)
            0   - trait_solve::wait (37 calls)
        0   - crate_def_map:wait (2 calls)
        0   - deref_by_trait (1 calls)
        0   - item::Builder::build (17 calls)
        0   - resolve_obligations_as_possible (2 calls)
change: 62.59µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
   37ms - dot completion
       23ms - CompletionContext::new
           11ms - parse_query @ FileId(45)
            5ms - CompletionContext::analyze (1 calls)
            5ms - CompletionContext::expand (1 calls)
            0   - Semantics::analyze_impl (1 calls)
            0   - crate_def_map:wait (1 calls)
        9ms - import_on_the_fly_method @ 
            9ms - import_assets::search_for_imports
                9ms - import_assets::search_for
                    9ms - import_assets::trait_applicable_items
                        0   - applicable_inherent_traits (1 calls)
                        0   - env_traits (1 calls)
                        3ms - get_name_definition (40 calls)
                        3ms - items_with_name (1 calls)
                        3ms - iterate_method_candidates (1 calls)
                    0   - Semantics::analyze_impl (1 calls)
                    0   - import_assets::scope_definitions (1 calls)
        0   - crate_def_map:wait (2 calls)
        0   - deref_by_trait (1 calls)
        0   - item::Builder::build (11 calls)
        4ms - iterate_method_candidates (1 calls)
        0   - lang_item_query (2 calls)
        0   - resolve_obligations_as_possible (2 calls)
test integrated_benchmarks::integrated_completion_benchmark ... ok

# -git salsa
running 1 test
workspace loading: 7.44s
initial: 1.31s
change: 51.48µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
   95ms - unqualified path completion
       23ms - CompletionContext::new
           10ms - parse_query @ FileId(49)
            5ms - CompletionContext::analyze (1 calls)
            5ms - CompletionContext::expand (1 calls)
            0   - Semantics::analyze_impl (1 calls)
            0   - crate_def_map:wait (1 calls)
       41ms - import_on_the_fly @ sel
           22ms - import_assets::search_for_imports
               22ms - import_assets::search_for
                   22ms - import_assets::path_applicable_imports
                       22ms - items_with_name @ Name: sel, crate: Exclude, assoc items: Some("hir"), limit: Some(40)
                           22ms - find_items
                               21ms - crate_symbols
                                    0   - crate_def_map:wait (1 calls)
                                   21ms - module_symbols (11 calls)
                                0   - query_external_importables (1 calls)
                                0   - symbol_index::Query::search (1 calls)
                        0   - find_path_prefixed (45 calls)
                        0   - get_name_definition (40 calls)
                    0   - Semantics::analyze_impl (1 calls)
                    0   - import_assets::scope_definitions (1 calls)
            6ms - render_resolution
                6ms - render_fn
                    1ms - body_with_source_map_query (1 calls)
                    0   - crate_def_map:wait (5 calls)
                    0   - generic_params_query (1 calls)
                    4ms - infer:wait (1 calls)
                    0   - lang_item_query (1 calls)
            0   - attrs_query (7 calls)
            0   - crate_def_map:wait (3 calls)
            0   - item::Builder::build (35 calls)
           11ms - render_resolution (34 calls)
       12ms - complete_expr_path
           12ms - CompletionContext::process_all_names
                0   - attrs_query (27 calls)
                0   - crate_def_map:wait (344 calls)
                0   - item::Builder::build (303 calls)
               11ms - render_resolution (303 calls)
            0   - item::Builder::build (13 calls)
       18ms - iterate_method_candidates
            0   - PerNs::filter_visibility (24 calls)
            0   - crate_def_map:wait (384 calls)
            0   - deref_by_trait (7 calls)
            0   - generic_params_query (210 calls)
            0   - item::Builder::build (134 calls)
           12ms - render_method (134 calls)
            0   - resolve_obligations_as_possible (17 calls)
            0   - trait_solve::wait (37 calls)
            5ms - ???
        0   - crate_def_map:wait (2 calls)
        0   - deref_by_trait (1 calls)
        0   - item::Builder::build (17 calls)
        0   - resolve_obligations_as_possible (2 calls)
change: 47.79µs
cpu profiling is disabled, uncomment `default = [ "cpu_profiler" ]` in Cargo.toml to enable.
   39ms - dot completion
       27ms - CompletionContext::new
           10ms - parse_query @ FileId(49)
            9ms - CompletionContext::analyze
                9ms - Semantics::analyze_impl
                    9ms - infer:wait @ name
                        9ms - infer_query
                            0   - crate_def_map:wait (9 calls)
                            0   - deref_by_trait (13 calls)
                            3ms - resolve_obligations_as_possible (74 calls)
                            0   - trait_solve::wait (4 calls)
                            5ms - ???
                    0   - SourceBinder::to_module_def (1 calls)
                    0   - body_with_source_map_query (1 calls)
                    0   - crate_def_map:wait (1 calls)
            5ms - CompletionContext::expand (1 calls)
            0   - Semantics::analyze_impl (1 calls)
            0   - crate_def_map:wait (1 calls)
        7ms - import_on_the_fly_method @ 
            7ms - import_assets::search_for_imports
                7ms - import_assets::search_for
                    7ms - import_assets::trait_applicable_items
                        0   - applicable_inherent_traits (1 calls)
                        0   - env_traits (1 calls)
                        0   - get_name_definition (40 calls)
                        3ms - items_with_name (1 calls)
                        3ms - iterate_method_candidates (1 calls)
                    0   - Semantics::analyze_impl (1 calls)
                    0   - import_assets::scope_definitions (1 calls)
        0   - crate_def_map:wait (2 calls)
        0   - deref_by_trait (1 calls)
        0   - item::Builder::build (11 calls)
        4ms - iterate_method_candidates (1 calls)
        0   - lang_item_query (2 calls)
        0   - resolve_obligations_as_possible (2 calls)

So it's still slightly slower, but not that bad.

[Veykril]

Huh, it was slower by a lot when I tested this back then, but I guess we can try and see.

[lnicola]

Yeah, but then Niko made some changes and brought it within 10% of the original, according to that Zulip topic.

[bors]

☔ The latest upstream changes (presumably #14843) made this pull request unmergeable. Please resolve the merge conflicts.

[HKalbasi]

I tried to see the compile time effect of this branch using cargo build --timings and it seems the syn2 is not on the critical path. The current critical path is quote > syn1 > salsa-macros > salsa > base-db > hir-expand > hir-def > hir-ty and the maximum path including syn2 is quote > syn2 > chalk-derive > chalk-ir > chalk-solve > chalk-recursive > hir-ty which has ~4 seconds slack time. And the reported time by cargo on my machine is 1m 15s this branch vs 1m 13s on master. The situation is probably worse in CI which has less cores than my machine, but it shouldn't be that bad as the syn takes 12s to compile so I guess a ~6 delay? Let's see.

Is there any other problem in mixing syns other than compile time?

[HKalbasi]

The compile test time on ubuntu in CI is 10m 4s on this branch. On #15003 it is 11m 31, and on #14990 it is 9m 44s so I guess it is too unstable to say anything based on that.

[lnicola]

I hope we could eat the syn 2 slowdown for now, because there's some great goodies coming up in chalk.

[Veykril]

I agree, let's just upgrade chalk given all the recent changes that landed there. I didn't realize that syn and syn2 will probably build in parallel, since almost everything is blocked on syn building, so the slow down won't be too big after all.

[HKalbasi]

@bors r+

[bors]

📌 Commit 527dfed has been approved by HKalbasi

It is now in the queue for this repository.

[bors]

⌛ Testing commit 527dfed with merge 0cad484...

[bors]

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing 0cad484 to master...