diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index 935c547b0e..8f27a7fc9d 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -122,16 +122,27 @@ pub fn expand_hypothetical( hypothetical_args: &ast::TokenTree, token_to_map: SyntaxToken, ) -> Option<(SyntaxNode, SyntaxToken)> { - let macro_file = MacroFile { macro_call_id: actual_macro_call }; let (tt, tmap_1) = mbe::syntax_node_to_token_tree(hypothetical_args.syntax()); let range = token_to_map.text_range().checked_sub(hypothetical_args.syntax().text_range().start())?; let token_id = tmap_1.token_by_range(range)?; - let macro_def = expander(db, actual_macro_call)?; - let hypothetical_expansion = - macro_expand_with_arg(db, macro_file.macro_call_id, Some(Arc::new((tt, tmap_1)))); - let (node, tmap_2) = expansion_to_syntax(db, macro_file, hypothetical_expansion).value?; + let lazy_id = match actual_macro_call { + MacroCallId::LazyMacro(id) => id, + MacroCallId::EagerMacro(_) => return None, + }; + + let macro_def = { + let loc = db.lookup_intern_macro(lazy_id); + db.macro_def(loc.def)? + }; + + let hypothetical_expansion = macro_def.expand(db, lazy_id, &tt); + + let fragment_kind = to_fragment_kind(db, actual_macro_call); + + let (node, tmap_2) = + mbe::token_tree_to_syntax_node(&hypothetical_expansion.value, fragment_kind).ok()?; let token_id = macro_def.map_id_down(token_id); let range = tmap_2.range_by_token(token_id)?.by_kind(token_to_map.kind())?; @@ -156,17 +167,9 @@ fn parse_or_expand(db: &dyn AstDatabase, file_id: HirFileId) -> Option ExpandResult, Arc)>> { - let result = db.macro_expand(macro_file.macro_call_id); - expansion_to_syntax(db, macro_file, result) -} - -fn expansion_to_syntax( - db: &dyn AstDatabase, - macro_file: MacroFile, - result: ExpandResult>>, ) -> ExpandResult, Arc)>> { let _p = profile::span("parse_macro_expansion"); + let result = db.macro_expand(macro_file.macro_call_id); if let Some(err) = &result.err { // Note: @@ -309,19 +312,6 @@ fn macro_expand_error(db: &dyn AstDatabase, macro_call: MacroCallId) -> Option Option> { - let lazy_id = match id { - MacroCallId::LazyMacro(id) => id, - MacroCallId::EagerMacro(_id) => { - return None; - } - }; - - let loc = db.lookup_intern_macro(lazy_id); - let macro_rules = db.macro_def(loc.def)?; - Some(macro_rules) -} - fn macro_expand_with_arg( db: &dyn AstDatabase, id: MacroCallId, diff --git a/docs/dev/style.md b/docs/dev/style.md index 6ab60b50ed..00de7a7117 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -449,6 +449,39 @@ fn query_all(name: String, case_sensitive: bool) -> Vec { ... } fn query_first(name: String, case_sensitive: bool) -> Option { ... } ``` +## Prefer Separate Functions Over Parameters + +If a function has a `bool` or an `Option` parameter, and it is always called with `true`, `false`, `Some` and `None` literals, split the function in two. + +```rust +// GOOD +fn caller_a() { + foo() +} + +fn caller_b() { + foo_with_bar(Bar::new()) +} + +fn foo() { ... } +fn foo_with_bar(bar: Bar) { ... } + +// BAD +fn caller_a() { + foo(None) +} + +fn caller_b() { + foo(Some(Bar::new())) +} + +fn foo(bar: Option) { ... } +``` + +**Rationale:** more often than not, such functions display "`false sharing`" -- they have additional `if` branching inside for two different cases. +Splitting the two different control flows into two functions simplifies each path, and remove cross-dependencies between the two paths. +If there's common code between `foo` and `foo_with_bar`, extract *that* into a common helper. + ## Avoid Monomorphization Avoid making a lot of code type parametric, *especially* on the boundaries between crates.