From 5046889f431e4eb94a1add6a23f023ae5e461a16 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 6 Sep 2023 18:52:12 +0200 Subject: [PATCH] Don't allocate the format_args template string as an expression --- crates/hir-def/src/body/lower.rs | 50 +++++++++---------- crates/hir-def/src/body/tests.rs | 6 +-- crates/hir-def/src/hir/format_args.rs | 15 ++---- .../extract_expressions_from_format_string.rs | 6 --- 4 files changed, 31 insertions(+), 46 deletions(-) diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 9e2b6f1a864..38073ce3da5 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -32,7 +32,7 @@ hir::{ dummy_expr_id, format_args::{ - self, FormatAlignment, FormatArgsPiece, FormatArgument, FormatArgumentKind, + self, FormatAlignment, FormatArgs, FormatArgsPiece, FormatArgument, FormatArgumentKind, FormatArgumentsCollector, FormatCount, FormatDebugHex, FormatOptions, FormatPlaceholder, FormatSign, FormatTrait, }, @@ -1568,6 +1568,24 @@ fn with_opt_labeled_rib( // endregion: labels // region: format + fn expand_macros_to_string(&mut self, expr: ast::Expr) -> Option<(ast::String, bool)> { + let m = match expr { + ast::Expr::MacroExpr(m) => m, + ast::Expr::Literal(l) => { + return match l.kind() { + ast::LiteralKind::String(s) => Some((s, true)), + _ => None, + } + } + _ => return None, + }; + let e = m.macro_call()?; + let macro_ptr = AstPtr::new(&e); + let (exp, _) = self.collect_macro_call(e, macro_ptr, true, |this, expansion| { + expansion.and_then(|it| this.expand_macros_to_string(it)) + })?; + Some((exp, false)) + } fn collect_format_args( &mut self, @@ -1586,31 +1604,13 @@ fn collect_format_args( }); let template = f.template(); let fmt_snippet = template.as_ref().map(ToString::to_string); - - // FIXME: We shouldn't allocate this one, just resolve and expand the macros to fetch the - // string literal! - let expr = self.collect_expr_opt(template); - - let fmt = 'b: { - if let Expr::Literal(Literal::String(_)) = self.body[expr] { - let source = self.source_map.expr_map_back[expr].clone(); - let is_direct_literal = source.file_id == self.expander.current_file_id; - if let ast::Expr::Literal(l) = - source.value.to_node(&self.db.parse_or_expand(source.file_id)) - { - if let ast::LiteralKind::String(s) = l.kind() { - break 'b format_args::parse( - expr, - &s, - fmt_snippet, - args, - is_direct_literal, - |name| self.alloc_expr_desugared(Expr::Path(Path::from(name))), - ); - } - } + let fmt = match template.and_then(|it| self.expand_macros_to_string(it)) { + Some((s, is_direct_literal)) => { + format_args::parse(&s, fmt_snippet, args, is_direct_literal, |name| { + self.alloc_expr_desugared(Expr::Path(Path::from(name))) + }) } - return self.missing_expr(); + None => FormatArgs { template: Default::default(), arguments: args.finish() }, }; // Create a list of all _unique_ (argument, format trait) combinations. diff --git a/crates/hir-def/src/body/tests.rs b/crates/hir-def/src/body/tests.rs index 76880bf68ed..1658757d2b6 100644 --- a/crates/hir-def/src/body/tests.rs +++ b/crates/hir-def/src/body/tests.rs @@ -161,7 +161,7 @@ fn main() { let count = 10; builtin#lang(Arguments::new_v1_formatted)( &[ - "\"hello ", " ", " friends, we ", " ", "", "\"", + "\"hello ", " ", " friends, we ", " ", "", "\"", ], &[ builtin#lang(Argument::new_display)( @@ -172,7 +172,7 @@ fn main() { &are, ), builtin#lang(Argument::new_display)( &"!", - ), + ), ], &[ builtin#lang(Placeholder::new)( @@ -212,7 +212,7 @@ fn main() { 0u32, builtin#lang(Count::Implied), builtin#lang(Count::Implied), - ), + ), ], unsafe { builtin#lang(UnsafeArg::new)() diff --git a/crates/hir-def/src/hir/format_args.rs b/crates/hir-def/src/hir/format_args.rs index 6197ee16494..d8f8e6026a5 100644 --- a/crates/hir-def/src/hir/format_args.rs +++ b/crates/hir-def/src/hir/format_args.rs @@ -1,3 +1,4 @@ +//! Parses `format_args` input. use std::mem; use hir_expand::name::Name; @@ -12,7 +13,6 @@ #[derive(Debug, Clone, PartialEq, Eq)] pub struct FormatArgs { - pub template_expr: ExprId, pub template: Box<[FormatArgsPiece]>, pub arguments: FormatArguments, } @@ -166,7 +166,6 @@ enum PositionUsedAs { use PositionUsedAs::*; pub(crate) fn parse( - expr: ExprId, s: &ast::String, fmt_snippet: Option, mut args: FormatArgumentsCollector, @@ -195,11 +194,7 @@ pub(crate) fn parse( let is_source_literal = parser.is_source_literal; if !parser.errors.is_empty() { // FIXME: Diagnose - return FormatArgs { - template_expr: expr, - template: Default::default(), - arguments: args.finish(), - }; + return FormatArgs { template: Default::default(), arguments: args.finish() }; } let to_span = |inner_span: parse::InnerSpan| { @@ -419,11 +414,7 @@ enum ArgRef<'a> { // FIXME: Diagnose } - FormatArgs { - template_expr: expr, - template: template.into_boxed_slice(), - arguments: args.finish(), - } + FormatArgs { template: template.into_boxed_slice(), arguments: args.finish() } } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs b/crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs index 25c4b49c945..31a1ff496e1 100644 --- a/crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs +++ b/crates/ide-assists/src/handlers/extract_expressions_from_format_string.rs @@ -34,23 +34,17 @@ pub(crate) fn extract_expressions_from_format_string( let fmt_string = ctx.find_token_at_offset::()?; let tt = fmt_string.syntax().parent().and_then(ast::TokenTree::cast)?; - dbg!(); let expanded_t = ast::String::cast( ctx.sema.descend_into_macros_with_kind_preference(fmt_string.syntax().clone(), 0.into()), )?; - dbg!(); if !is_format_string(&expanded_t) { - dbg!(); return None; } - dbg!(); let (new_fmt, extracted_args) = parse_format_exprs(fmt_string.text()).ok()?; - dbg!(); if extracted_args.is_empty() { return None; } - dbg!(); acc.add( AssistId(