Stop using LinkedGraph in lexical_region_resolve#153012
Stop using LinkedGraph in lexical_region_resolve#153012rust-bors[bot] merged 1 commit intorust-lang:mainfrom
LinkedGraph in lexical_region_resolve#153012Conversation
|
r? @fmease rustbot has assigned @fmease. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
I don't expect this to be on a hot path for perf, but let's check. @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Stop using `LinkedGraph` in `lexical_region_resolve`
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (71438fe): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary 1.8%, secondary 2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -1.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 480.206s -> 480.109s (-0.02%) |
|
@bors rollup=maybe |
compiler/rustc_infer/src/infer/lexical_region_resolve/indexed_edges.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/lexical_region_resolve/indexed_edges.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/lexical_region_resolve/indexed_edges.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/lexical_region_resolve/indexed_edges.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/lexical_region_resolve/indexed_edges.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_infer/src/infer/lexical_region_resolve/indexed_edges.rs
Outdated
Show resolved
Hide resolved
|
This makes sense if the goal is to eliminate |
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| // Only push a var out-edge for `VarSub...` constraints. | ||
| match c.kind { | ||
| ConstraintKind::VarSubVar | ConstraintKind::VarSubReg => { | ||
| out_edges[c.sub.as_var()].push(pair) | ||
| } | ||
| ConstraintKind::RegSubVar | ConstraintKind::RegSubReg => {} | ||
| } |
There was a problem hiding this comment.
I applied the suggestions, but to me this new version seems more confusing, because the visual pattern from before is now obscured.
|
Thanks! |
Stop using `LinkedGraph` in `lexical_region_resolve` There are only two users of the older `LinkedGraph` data structure, and this is one. It turns out that this diagnostic-related code doesn't need any non-trivial graph operations (since it does its own graph traversal); it just needs the ability to get a list of in-edges or out-edges (constraints) for any particular node. That's easy enough to do with a simple custom data structure. Inspired by rust-lang#152621, which wants to make changes to `LinkedGraph` that wouldn't make sense for this use-site.
Stop using `LinkedGraph` in `lexical_region_resolve` There are only two users of the older `LinkedGraph` data structure, and this is one. It turns out that this diagnostic-related code doesn't need any non-trivial graph operations (since it does its own graph traversal); it just needs the ability to get a list of in-edges or out-edges (constraints) for any particular node. That's easy enough to do with a simple custom data structure. Inspired by rust-lang#152621, which wants to make changes to `LinkedGraph` that wouldn't make sense for this use-site.
Rollup of 7 pull requests Successful merges: - #151143 (explicit tail calls: support indirect arguments) - #153012 (Stop using `LinkedGraph` in `lexical_region_resolve`) - #150828 (Improved security section in rustdoc for `current_exe`) - #153117 (Remove mutation from macro path URL construction) - #153128 (Recover feature lang_items for emscripten) - #153138 (Print path root when printing path) - #153159 (Work around a false `err.emit();` type error in rust-analyzer)
Stop using `LinkedGraph` in `lexical_region_resolve` There are only two users of the older `LinkedGraph` data structure, and this is one. It turns out that this diagnostic-related code doesn't need any non-trivial graph operations (since it does its own graph traversal); it just needs the ability to get a list of in-edges or out-edges (constraints) for any particular node. That's easy enough to do with a simple custom data structure. Inspired by rust-lang#152621, which wants to make changes to `LinkedGraph` that wouldn't make sense for this use-site.
Stop using `LinkedGraph` in `lexical_region_resolve` There are only two users of the older `LinkedGraph` data structure, and this is one. It turns out that this diagnostic-related code doesn't need any non-trivial graph operations (since it does its own graph traversal); it just needs the ability to get a list of in-edges or out-edges (constraints) for any particular node. That's easy enough to do with a simple custom data structure. Inspired by rust-lang#152621, which wants to make changes to `LinkedGraph` that wouldn't make sense for this use-site.
…uwer Rollup of 11 pull requests Successful merges: - #151431 (Add new unstable attribute: `#[export_visibility = ...]`.) - #153012 (Stop using `LinkedGraph` in `lexical_region_resolve`) - #153179 (Force a CI LLVM stamp bump) - #150828 (Improved security section in rustdoc for `current_exe`) - #152673 (rustc_public: rewrite `bridge_impl` to reduce boilerplate) - #152674 (rustc_public: remove the `CrateDefItems` trait) - #153073 (Fix mem::conjure_zst panic message to use any::type_name instead) - #153117 (Remove mutation from macro path URL construction) - #153128 (Recover feature lang_items for emscripten) - #153138 (Print path root when printing path) - #153159 (Work around a false `err.emit();` type error in rust-analyzer)
Stop using `LinkedGraph` in `lexical_region_resolve` There are only two users of the older `LinkedGraph` data structure, and this is one. It turns out that this diagnostic-related code doesn't need any non-trivial graph operations (since it does its own graph traversal); it just needs the ability to get a list of in-edges or out-edges (constraints) for any particular node. That's easy enough to do with a simple custom data structure. Inspired by rust-lang#152621, which wants to make changes to `LinkedGraph` that wouldn't make sense for this use-site.
…uwer Rollup of 12 pull requests Successful merges: - #151143 (explicit tail calls: support indirect arguments) - #153012 (Stop using `LinkedGraph` in `lexical_region_resolve`) - #153175 (Clarify a confusing green-path function) - #153179 (Force a CI LLVM stamp bump) - #150828 (Improved security section in rustdoc for `current_exe`) - #152673 (rustc_public: rewrite `bridge_impl` to reduce boilerplate) - #152674 (rustc_public: remove the `CrateDefItems` trait) - #153073 (Fix mem::conjure_zst panic message to use any::type_name instead) - #153117 (Remove mutation from macro path URL construction) - #153128 (Recover feature lang_items for emscripten) - #153138 (Print path root when printing path) - #153159 (Work around a false `err.emit();` type error in rust-analyzer)
…uwer Rollup of 12 pull requests Successful merges: - #151143 (explicit tail calls: support indirect arguments) - #153012 (Stop using `LinkedGraph` in `lexical_region_resolve`) - #153175 (Clarify a confusing green-path function) - #153179 (Force a CI LLVM stamp bump) - #150828 (Improved security section in rustdoc for `current_exe`) - #152673 (rustc_public: rewrite `bridge_impl` to reduce boilerplate) - #152674 (rustc_public: remove the `CrateDefItems` trait) - #153073 (Fix mem::conjure_zst panic message to use any::type_name instead) - #153117 (Remove mutation from macro path URL construction) - #153128 (Recover feature lang_items for emscripten) - #153138 (Print path root when printing path) - #153159 (Work around a false `err.emit();` type error in rust-analyzer)
Rollup merge of #153012 - Zalathar:lexical, r=petrochenkov Stop using `LinkedGraph` in `lexical_region_resolve` There are only two users of the older `LinkedGraph` data structure, and this is one. It turns out that this diagnostic-related code doesn't need any non-trivial graph operations (since it does its own graph traversal); it just needs the ability to get a list of in-edges or out-edges (constraints) for any particular node. That's easy enough to do with a simple custom data structure. Inspired by #152621, which wants to make changes to `LinkedGraph` that wouldn't make sense for this use-site.
There are only two users of the older
LinkedGraphdata structure, and this is one.It turns out that this diagnostic-related code doesn't need any non-trivial graph operations (since it does its own graph traversal); it just needs the ability to get a list of in-edges or out-edges (constraints) for any particular node. That's easy enough to do with a simple custom data structure.
Inspired by #152621, which wants to make changes to
LinkedGraphthat wouldn't make sense for this use-site.