mirror of
https://github.com/rust-lang/rust-analyzer
synced 2024-12-26 04:53:34 +00:00
simplify macro expansion code
Using `Option` arguments such that you always pass `None` or `Some` at the call site is a code smell.
This commit is contained in:
parent
95dc8ef265
commit
3f6980e4e1
2 changed files with 50 additions and 27 deletions
|
@ -122,16 +122,27 @@ pub fn expand_hypothetical(
|
||||||
hypothetical_args: &ast::TokenTree,
|
hypothetical_args: &ast::TokenTree,
|
||||||
token_to_map: SyntaxToken,
|
token_to_map: SyntaxToken,
|
||||||
) -> Option<(SyntaxNode, 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 (tt, tmap_1) = mbe::syntax_node_to_token_tree(hypothetical_args.syntax());
|
||||||
let range =
|
let range =
|
||||||
token_to_map.text_range().checked_sub(hypothetical_args.syntax().text_range().start())?;
|
token_to_map.text_range().checked_sub(hypothetical_args.syntax().text_range().start())?;
|
||||||
let token_id = tmap_1.token_by_range(range)?;
|
let token_id = tmap_1.token_by_range(range)?;
|
||||||
let macro_def = expander(db, actual_macro_call)?;
|
|
||||||
|
|
||||||
let hypothetical_expansion =
|
let lazy_id = match actual_macro_call {
|
||||||
macro_expand_with_arg(db, macro_file.macro_call_id, Some(Arc::new((tt, tmap_1))));
|
MacroCallId::LazyMacro(id) => id,
|
||||||
let (node, tmap_2) = expansion_to_syntax(db, macro_file, hypothetical_expansion).value?;
|
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 token_id = macro_def.map_id_down(token_id);
|
||||||
let range = tmap_2.range_by_token(token_id)?.by_kind(token_to_map.kind())?;
|
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<SyntaxNod
|
||||||
fn parse_macro_expansion(
|
fn parse_macro_expansion(
|
||||||
db: &dyn AstDatabase,
|
db: &dyn AstDatabase,
|
||||||
macro_file: MacroFile,
|
macro_file: MacroFile,
|
||||||
) -> ExpandResult<Option<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)>> {
|
|
||||||
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<Option<Arc<tt::Subtree>>>,
|
|
||||||
) -> ExpandResult<Option<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)>> {
|
) -> ExpandResult<Option<(Parse<SyntaxNode>, Arc<mbe::TokenMap>)>> {
|
||||||
let _p = profile::span("parse_macro_expansion");
|
let _p = profile::span("parse_macro_expansion");
|
||||||
|
let result = db.macro_expand(macro_file.macro_call_id);
|
||||||
|
|
||||||
if let Some(err) = &result.err {
|
if let Some(err) = &result.err {
|
||||||
// Note:
|
// Note:
|
||||||
|
@ -309,19 +312,6 @@ fn macro_expand_error(db: &dyn AstDatabase, macro_call: MacroCallId) -> Option<E
|
||||||
db.macro_expand(macro_call).err
|
db.macro_expand(macro_call).err
|
||||||
}
|
}
|
||||||
|
|
||||||
fn expander(db: &dyn AstDatabase, id: MacroCallId) -> Option<Arc<TokenExpander>> {
|
|
||||||
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(
|
fn macro_expand_with_arg(
|
||||||
db: &dyn AstDatabase,
|
db: &dyn AstDatabase,
|
||||||
id: MacroCallId,
|
id: MacroCallId,
|
||||||
|
|
|
@ -449,6 +449,39 @@ fn query_all(name: String, case_sensitive: bool) -> Vec<Item> { ... }
|
||||||
fn query_first(name: String, case_sensitive: bool) -> Option<Item> { ... }
|
fn query_first(name: String, case_sensitive: bool) -> Option<Item> { ... }
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## 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<Bar>) { ... }
|
||||||
|
```
|
||||||
|
|
||||||
|
**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 Monomorphization
|
||||||
|
|
||||||
Avoid making a lot of code type parametric, *especially* on the boundaries between crates.
|
Avoid making a lot of code type parametric, *especially* on the boundaries between crates.
|
||||||
|
|
Loading…
Reference in a new issue