Skip to content

Commit f2fbc8e

Browse files
committed
Don't attempt fallback if return type has non-static lifetime
1 parent 79c2eac commit f2fbc8e

File tree

3 files changed

+85
-17
lines changed

3 files changed

+85
-17
lines changed

components/salsa-macros/src/tracked_fn.rs

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ impl Macro {
9191
ty
9292
});
9393
let output_ty = self.output_ty(&db_lt, &item)?;
94+
let mut has_lifetime_visitor = HasLifetimeVisitor {
95+
has_lifetime: false,
96+
};
97+
syn::visit::visit_type(&mut has_lifetime_visitor, &output_ty);
98+
let has_lifetime = has_lifetime_visitor.has_lifetime;
99+
94100
let (cycle_recovery_fn, cycle_recovery_initial, cycle_recovery_strategy) =
95101
self.cycle_recovery()?;
96102
let is_specifiable = self.args.specify.is_some();
@@ -184,11 +190,23 @@ impl Macro {
184190
UpdateDispatch::<#output_ty>::maybe_update
185191
};
186192
let assert_return_type_is_update = if requires_update {
187-
quote! {
188-
#[allow(clippy::all, warnings)]
189-
fn _assert_return_type_is_update<#db_lt>() {
190-
use #zalsa::{UpdateFallback, UpdateDispatch};
191-
#maybe_update_path;
193+
if has_lifetime {
194+
// don't import the `UpdateFallback` if we have non-static
195+
// lifetimes to avoid confusing errors
196+
quote! {
197+
#[allow(clippy::all, warnings)]
198+
fn _assert_return_type_is_update<#db_lt>() {
199+
fn _assert_is_update<T: #zalsa::Update>() { }
200+
_assert_is_update::<#output_ty>();
201+
}
202+
}
203+
} else {
204+
quote! {
205+
#[allow(clippy::all, warnings)]
206+
fn _assert_return_type_is_update<#db_lt>() {
207+
use #zalsa::{UpdateFallback, UpdateDispatch};
208+
#maybe_update_path;
209+
}
192210
}
193211
}
194212
} else {
@@ -307,6 +325,19 @@ impl syn::visit_mut::VisitMut for ToDbLifetimeVisitor {
307325
}
308326
}
309327

328+
struct HasLifetimeVisitor {
329+
has_lifetime: bool,
330+
}
331+
332+
impl<'ast> syn::visit::Visit<'ast> for HasLifetimeVisitor {
333+
fn visit_lifetime(&mut self, l: &'ast syn::Lifetime) {
334+
// We don't consider `'static` to be a lifetime in this context.
335+
if l.ident != "static" {
336+
self.has_lifetime = true;
337+
}
338+
}
339+
}
340+
310341
#[derive(Debug, PartialEq, Eq, Hash)]
311342
enum FunctionType {
312343
Constant,

src/update.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ fallback_impl! { compact_str::CompactString, }
494494

495495
macro_rules! tuple_impl {
496496
($($t:ident),*; $($u:ident),*) => {
497+
#[diagnostic::do_not_recommend]
497498
unsafe impl<$($t),*> Update for ($($t,)*)
498499
where
499500
$($t: Update,)*

tests/compile-fail/tracked_fn_return_ref.stderr

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,26 +29,62 @@ warning: elided lifetime has a name
2929
43 | ) -> ContainsRef<'_> {
3030
| ^^ this elided lifetime gets resolved as `'db`
3131

32-
error: lifetime may not live long enough
32+
error[E0277]: the trait bound `&'db str: Update` is not satisfied
3333
--> tests/compile-fail/tracked_fn_return_ref.rs:15:67
3434
|
3535
15 | fn tracked_fn_return_ref<'db>(db: &'db dyn Db, input: MyInput) -> &'db str {
36-
| --- lifetime `'db` defined here ^ requires that `'db` must outlive `'static`
36+
| ^^^^^^^^ the trait `Update` is not implemented for `&'db str`
37+
|
38+
= help: the trait `Update` is implemented for `String`
39+
note: required by a bound in `<tracked_fn_return_ref::Configuration_ as salsa::function::Configuration>::execute::_assert_return_type_is_update::_assert_is_update`
40+
--> tests/compile-fail/tracked_fn_return_ref.rs:14:1
41+
|
42+
14 | #[salsa::tracked]
43+
| ^^^^^^^^^^^^^^^^^ required by this bound in `_assert_is_update`
44+
= note: this error originates in the attribute macro `salsa::tracked` (in Nightly builds, run with -Z macro-backtrace for more info)
3745

38-
error: lifetime may not live long enough
46+
error[E0277]: the trait bound `ContainsRef<'db>: Update` is not satisfied
3947
--> tests/compile-fail/tracked_fn_return_ref.rs:23:6
4048
|
41-
20 | fn tracked_fn_return_struct_containing_ref<'db>(
42-
| --- lifetime `'db` defined here
43-
...
4449
23 | ) -> ContainsRef<'db> {
45-
| ^^^^^^^^^^^ requires that `'db` must outlive `'static`
50+
| ^^^^^^^^^^^^^^^^ the trait `Update` is not implemented for `ContainsRef<'db>`
51+
|
52+
= help: the following other types implement trait `Update`:
53+
Arc<T>
54+
BTreeMap<K, V>
55+
BTreeSet<K>
56+
Box<T>
57+
Box<[T]>
58+
HashMap<K, V, S>
59+
HashSet<K, S>
60+
Infallible
61+
and $N others
62+
note: required by a bound in `<tracked_fn_return_struct_containing_ref::Configuration_ as salsa::function::Configuration>::execute::_assert_return_type_is_update::_assert_is_update`
63+
--> tests/compile-fail/tracked_fn_return_ref.rs:19:1
64+
|
65+
19 | #[salsa::tracked]
66+
| ^^^^^^^^^^^^^^^^^ required by this bound in `_assert_is_update`
67+
= note: this error originates in the attribute macro `salsa::tracked` (in Nightly builds, run with -Z macro-backtrace for more info)
4668

47-
error: lifetime may not live long enough
69+
error[E0277]: the trait bound `ContainsRef<'db>: Update` is not satisfied
4870
--> tests/compile-fail/tracked_fn_return_ref.rs:43:6
4971
|
50-
40 | fn tracked_fn_return_struct_containing_ref_elided_explicit<'db>(
51-
| --- lifetime `'db` defined here
52-
...
5372
43 | ) -> ContainsRef<'_> {
54-
| ^^^^^^^^^^^ requires that `'db` must outlive `'static`
73+
| ^^^^^^^^^^^^^^^ the trait `Update` is not implemented for `ContainsRef<'db>`
74+
|
75+
= help: the following other types implement trait `Update`:
76+
Arc<T>
77+
BTreeMap<K, V>
78+
BTreeSet<K>
79+
Box<T>
80+
Box<[T]>
81+
HashMap<K, V, S>
82+
HashSet<K, S>
83+
Infallible
84+
and $N others
85+
note: required by a bound in `<tracked_fn_return_struct_containing_ref_elided_explicit::Configuration_ as salsa::function::Configuration>::execute::_assert_return_type_is_update::_assert_is_update`
86+
--> tests/compile-fail/tracked_fn_return_ref.rs:39:1
87+
|
88+
39 | #[salsa::tracked]
89+
| ^^^^^^^^^^^^^^^^^ required by this bound in `_assert_is_update`
90+
= note: this error originates in the attribute macro `salsa::tracked` (in Nightly builds, run with -Z macro-backtrace for more info)

0 commit comments

Comments
 (0)