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

inconsistent behavior with respect to lifetime elision #519

Closed
nikomatsakis opened this issue Jul 19, 2024 · 0 comments · Fixed by #596
Closed

inconsistent behavior with respect to lifetime elision #519

nikomatsakis opened this issue Jul 19, 2024 · 0 comments · Fixed by #596
Milestone

Comments

@nikomatsakis
Copy link
Member

In #518 the behavior with respect to lifetime elision for tracked functions is kind of inconsistent:

  • For the &dyn Db parameter, we always infer the elided lifetime to be 'db
  • For other random inputs, we basically disallow elision: if you use a '_, it will wind up in the value of an associated type, and give an error.
  • We'll also do one weird thing which is if you write fn foo<'db>(db: &dyn Db, x: Foo<'db>), I believe the db becomes &'db dyn Db.

In reality, there is only one lifetime you can correctly use, which is 'db -- salsa tracked functions must not take references except for a case like TrackedStruct<'db> or InternedStruct<'db>.

The current setup seems weird but there are two different choices we could make:

  • Introduce a 'db if it's not already there and replace all elided lifetimes with 'db.
  • Forbid elided lifetimes in the inputs unless there is exactly one (i.e., on the database). This is ~the current behavior, actually, except for the third bullet, which we should fix.

The former seems more convenient: there's only one thing you could want, so let's do it.

The latter is more consistent with non-tracked-functions, and it means if you remove the #[salsa::tracked], your function keeps compiling.

I'm inclined towards the latter for now.

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 a pull request may close this issue.

1 participant