Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

post const args in type dependent paths cleanup #74360

Closed
lcnr opened this issue Jul 15, 2020 · 1 comment · Fixed by #74404
Closed

post const args in type dependent paths cleanup #74360

lcnr opened this issue Jul 15, 2020 · 1 comment · Fixed by #74404
Labels
A-const-generics Area: const generics (parameters and arguments) C-enhancement Category: An issue proposing an enhancement or a PR with one. F-const_generics `#![feature(const_generics)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Jul 15, 2020

#74113 has some open FIXMEs

  • consider caching opt_const_param_of on disk (perf)
  • consider first calling tcx.def_kind(def_id) in opt_const_param_of to improve incremental
  • add a helper method for
if def.const_param_did.is_none() {
    if let const_param_did @ Some(_) = tcx.opt_const_param_of(def.did) {
        return tcx.query(ty::WithOptConstParam { const_param_did, ..def });
    }
}
@lcnr lcnr added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-const-generics Area: const generics (parameters and arguments) F-const_generics `#![feature(const_generics)]` labels Jul 15, 2020
@eddyb
Copy link
Member

eddyb commented Jul 15, 2020

Copying a couple of comments here:

  • Support const args in type dependent paths (Take 2) #74113 (comment) (src/librustc_typeck/collect/type_of.rs / fn opt_const_param_of)

    I think you can make this cheaper incremental-wise by checking tcx.def_kind(def_id) first, because that won't add a dependency to the entire definition pointed to by def_id, just the simple DefKind enum (but feel free to file this under further experimentation).

  • Support const args in type dependent paths (Take 2) #74113 (comment) (referring to the if ... { if let ... { return ... } } pattern)

    I just realized that this two-step process could probably be a helper method on WithOptConstParam (and do e.g. if let Some(def) = def.upgrade_with_const_param(tcx) or something like that, with the method documentation describing this kind of use pattern).

Manishearth added a commit to Manishearth/rust that referenced this issue Jul 20, 2020
…=eddyb

test caching opt_const_param_of on disc

Followup to rust-lang#74113, implements parts of rust-lang#74360

Tried caching `opt_const_param_of` on disk and adding an early exit if `tcx.dep_kind(def_id) != DefKind::AnonConst`.

Ended up causing a perf regression instead, so we just remove the FIXME and a short note to `opt_const_param_of`.

r? @eddyb
@bors bors closed this as completed in 4a86573 Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) C-enhancement Category: An issue proposing an enhancement or a PR with one. F-const_generics `#![feature(const_generics)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants