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

Provide a public API to obtain IngredientIndex #461

Merged

Conversation

Y-Nak
Copy link
Contributor

@Y-Nak Y-Nak commented Oct 8, 2023

Currently, It seems there is no public API that allows to obtain IngredientIndex from an ingredient itself.
The main motivation for this change is to make it possible to filter out cycle participants by IngredientIndex of a specific tracked function.

The one use case would be like the one below.

// NOTE: `lower_type_alias` and `lower_hir_ty` depend on each other.

#[salsa::tracked(return_ref, recovery_fn = recover_lower_type_alias_cycle)]
pub(crate) fn lower_type_alias(
    db: &dyn Db,
    alias: HirTypeAlias,
) -> Result<TyAlias, AliasCycle> {
    let hir_ty = alias.ty(db);
    let resolved_to = lower_hir_ty(db, hir_ty);

    Ok(TyAlias { resolved_to, ... })
}

#[salsa::tracked]
pub(crate) fn lower_hir_ty(db: &dyn Db, hir_ty: HirTy) -> TyId {
    ...
    // It turns out that `hir_ty` points to a type alias, so we need to lower it first.
    let ty_alias = lower_type_alias(db, alias).resolved_to;
    ...
}

fn recover_lower_type_alias_cycle(
    db: &dyn Db,
    cycle: &salsa::Cycle,
    _alias: HirTypeAlias,
) -> Result<TyAlias, AliasCycle> {
    // Finds ingredient index for `lower_type_alias` function to filter out the
    // cycle participants.
    let (jar, _): (&crate::Jar, _) = db.jar();
    let function =
        &<_ as salsa::storage::HasIngredientsFor<lower_type_alias>>::ingredient(jar).function;
    let ingredient_index = <FunctionIngredient<lower_type_alias> as Ingredient<
        dyn HirAnalysisDb,
    >>::ingredient_index(&function);

    // Here we want to find *ONLY* type aliases that are part of the cycle(i.e.,
    // ignore the `HirTy` in `lower_hir_ty` function).
    let alias_cycle = cycle
        .participant_keys()
        .filter_map(|key| {
            if ingredient_index == key.ingredient_index() {
                Some(lower_type_alias::key_from_id(key.key_index()))
            } else {
                None
            }
        })
        .collect();
    Err(AliasCycle(alias_cycle))
}

If this approach is Ok, I'd be happy to add some wrapper APIs for easy IngredientIndex retrieval.

@nikomatsakis
Copy link
Member

Hmm, @Y-Nak -- sorry for the delay -- I don't see an obvious problem with letting people get access to the IngredientIndex. To be honest, I don't have salsa in cache right now, though I am hoping to be able to come back to it next year and start hacking again. I'm going to merge this for now!

bors r+

@nikomatsakis
Copy link
Member

Whoops, I forget if we were using bors or what. :)

@nikomatsakis nikomatsakis added this pull request to the merge queue Nov 9, 2023
Merged via the queue into salsa-rs:master with commit 703794b Nov 9, 2023
3 checks passed
@Y-Nak Y-Nak deleted the provide-access-to-ingredient-index branch November 15, 2023 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants