Skip to content

Add 'db lifetime to BuiltinDeriveMacroId #23060

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.

Sign up for GitHub

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

Open
ada4a wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add 'db lifetime to BuiltinDeriveMacroId #23060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d091a6
misc improvements
ada4a Jul 20, 2026
46504bc
Allow having lifetimes on the `DynMap`
ChayimFriedman2 Aug 6, 2026
a1c1390
relax lifetimes
ada4a Jul 20, 2026
d5387cb
add lifetime to `BuiltinDeriveMacroId`
ada4a Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 20 additions & 22 deletions crates/hir-def/src/attrs/docs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//! and highlight injection).

use std::{
cell::LazyCell,
convert::Infallible,
ops::{ControlFlow, Range},
};
Expand Down Expand Up @@ -420,7 +421,21 @@ fn extend_with_attrs<'a, 'db>(
make_resolver: &dyn Fn() -> Resolver<'db>,
) {
// Lazily initialised when we first encounter a `#[doc = macro!()]`.
let mut expander: Option<DocMacroExpander<'db>> = None;
let mut expander = LazyCell::new(|| {

@ChayimFriedman2 ChayimFriedman2 Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? (LazyCell has an additional branch).

@ada4a ada4a Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly code clarity I'd say? LazyCell makes it very clear what's happening: a value that is initialized lazily.

LazyCell has an additional branch

You mean to check whether the value has been initialized or not? But I think Option::get_or_insert_with does a similar check for None, no?

Anyway I don't feel too strongly about this, happy to revert.

@ChayimFriedman2 ChayimFriedman2 Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LazyCell needs to check for reentrance.

I wouldn't oppose if this was written this way to begin with, but I do oppose changing it in an unrelated PR.

let resolver = make_resolver();
let def_map = resolver.top_level_def_map();
let recursion_limit = def_map.recursion_limit();
DocMacroExpander {
db,
krate,
macro_depth: file_id.macro_expansion_depth(db),
recursion_limit,
resolver,
file_id,
ast_id_map: file_id.ast_id_map(db),
span_map: file_id.span_map(db),
}
});

expand_cfg_attr_with_doc_comments::<_, Infallible>(
AttrDocCommentIter::from_syntax_node(node).filter(|attr| match attr {
Expand All @@ -441,27 +456,10 @@ fn extend_with_attrs<'a, 'db>(
&& let ast::LiteralKind::String(value) = value.kind()
{
result.extend_with_doc_attr(value, indent);
} else {
let exp = expander.get_or_insert_with(|| {
let resolver = make_resolver();
let def_map = resolver.top_level_def_map();
let recursion_limit = def_map.recursion_limit();
DocMacroExpander {
db,
krate,
macro_depth: file_id.macro_expansion_depth(db),
recursion_limit,
resolver,
file_id,
ast_id_map: file_id.ast_id_map(db),
span_map: file_id.span_map(db),
}
});
if let Some(expanded) =
expand_doc_expr_via_macro_pipeline(exp, value)
{
result.extend_with_unmapped_doc_str(&expanded, indent);
}
} else if let Some(expanded) =
expand_doc_expr_via_macro_pipeline(&mut expander, value)
{
result.extend_with_unmapped_doc_str(&expanded, indent);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/hir-def/src/builtin_derive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl BuiltinDeriveImplMethod {
pub fn trait_method(
self,
db: &dyn SourceDatabase,
impl_: BuiltinDeriveImplId,
impl_: BuiltinDeriveImplId<'_>,
) -> Option<FunctionId> {
let loc = impl_.loc(db);
let lang_items = crate::lang_item::lang_items(db, loc.krate(db));
Expand Down
Loading