Implement concat eager macro #3392

edwin0cheng · 2020-03-02T06:06:23Z

This PR implements the following things:

Add basic eager macro infrastructure by introducing EagerCallId such that the new MacroCallId is defined as :

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum MacroCallId {
    LazyMacro(LazyMacroId),
    EagerMacro(EagerMacroId),
}

Add concat! builtin macro.

edwin0cheng · 2020-03-02T16:33:48Z

@matklad It is ready to review now.

matklad · 2020-03-02T19:04:25Z

Haven't looked closely yet, but overall it looks good to me!

crates/ra_hir_expand/src/eager.rs

matklad · 2020-03-03T15:06:17Z

crates/ra_hir_expand/src/builtin_macro.rs

@@ -208,23 +245,44 @@ fn format_args_expand(
    Ok(expanded)
 }

+fn unquote_str(lit: &tt::Literal) -> Option<String> {
+    let lit = ast::make::tokens::literal(&lit.to_string());
+    let token = ast::String::cast(lit)?;


Long term, we probably should have a way to uquote a tt without going via AST, but this totally fine for the time being

When I implement this functions, I tried to find whether we have a util functions for unquoting string and I found this and other one inside ra_syntax (validate_literal). I would be happy to implement one but I don't know where should it in, Maybe ra_fmt ? Anyway it will be another PR.

matklad · 2020-03-03T15:10:08Z

crates/ra_hir_expand/src/eager.rs

+
+pub fn expand_eager_macro(
+    db: &impl AstDatabase,
+    macro_call: InFile<ast::MacroCall>,


Hm, now that i think about it, I guess the right primitive is eager expansion of macro argument. In format!("{}", 92) the first argument, "{}" is expanded eagerly (so you can do format!(concat!(file!(), ": {}"), 92)), but the rest are expanded normally.

Not sure what's best: merge the current approach with EagerMacroId and then try to migrate it to EagerMacroArgumentId, or to do the refactor before merging the PR.

I lean towards merging now and refactoring later though :)

How about this one ?

format!(concat!("{","}"), concat!("Hell","o"));

The second concat! here, itself, is expanded lazily

Um.. I think format! is not a good example here since it is not a eager macro itself. format_args (which format! use) would be a better one. This macro produces a value of type fmt::Arguments and if i understand correctly, to produce that value, all arguments inside format_args have to be expanded beforehand. Or I still miss something ?

I think format! is not a good example here since it is not a eager macro itself.

That's precisely my point: it's not the macro that is eager, it's the argument. For format, I believe the first arg is expanded eagerly, and the rest lazily. But I might be missing some details. It might make sense to consult rustc to check if first&rest args of format are treated differently .

edwin0cheng · 2020-03-03T17:16:06Z

Yeah, I moved the expand_eager_macro to top. I agreed we could merge this first and let refactor it later. :)

matklad · 2020-03-03T17:17:08Z

bors r+

3392: Implement concat eager macro r=matklad a=edwin0cheng This PR implements the following things: 1. Add basic eager macro infrastructure by introducing `EagerCallId` such that the new `MacroCallId` is defined as : ``` #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum MacroCallId { LazyMacro(LazyMacroId), EagerMacro(EagerMacroId), } ``` 2. Add `concat!` builtin macro. Co-authored-by: Edwin Cheng <[email protected]>

bors · 2020-03-03T17:18:46Z

Build failed

Rust (ubuntu-latest)

edwin0cheng · 2020-03-03T17:25:33Z

bors retry

bors · 2020-03-03T17:36:27Z

Build succeeded

lnicola · 2020-03-03T17:46:11Z

I think this broke analysis-stats:

thread 'main' panicked at 'Can't find MACRO_CALL@[147; 161) in AstIdMap:
[SyntaxNodePtr { range: [6; 217), kind: MACRO_CALL }, SyntaxNodePtr { range: [218; 335), kind: CONST_DEF }]', crates/ra_hir_expand/src/ast_id_map.rs:88:21
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/backtrace-0.3.44/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/git.colasdn.top-1ecc6299db9ec823/backtrace-0.3.44/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1053
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1428
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:204
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:224
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:470
  11: rust_begin_unwind
             at src/libstd/panicking.rs:378
  12: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:332
  13: ra_hir_expand::ast_id_map::AstIdMap::erased_ast_id
  14: ra_hir_expand::eager::eager_macro_recur
  15: ra_hir_expand::eager::expand_eager_macro
  16: <ra_hir_def::AstIdWithPath<ra_syntax::ast::generated::MacroCall> as ra_hir_def::AsMacroCall>::as_call_id
  17: <ra_hir_expand::InFile<&ra_syntax::ast::generated::MacroCall> as ra_hir_def::AsMacroCall>::as_call_id
  18: ra_hir_def::body::Expander::enter_expand
  19: ra_hir_def::data::collect_impl_items_in_macro
  20: ra_hir_def::data::collect_impl_items_in_macro
  21: ra_hir_def::data::collect_impl_items_in_macro

edwin0cheng · 2020-03-03T18:33:27Z

@lnicola yes, I know what happened here. We submit another PR to fix that.

[edit]
submitted #3429

edwin0cheng force-pushed the new-eager-concat branch 3 times, most recently from e1e69c4 to 84effda Compare March 2, 2020 16:19

edwin0cheng changed the title ~~[WIP] Implement concat eager macro~~ Implement concat eager macro Mar 2, 2020

edwin0cheng requested a review from matklad March 2, 2020 16:33

matklad reviewed Mar 3, 2020

View reviewed changes

edwin0cheng added 6 commits March 4, 2020 01:21

Add LazyMacroId

0d55454

Implement concat macro

1465cc0

Add test

fe04c28

Add doc about eager expansion name resolution

02b8650

Move pub function first

1e9a323

Fix for rebasing

4d5e80c

edwin0cheng force-pushed the new-eager-concat branch from cf830ae to 4d5e80c Compare March 3, 2020 17:24

bors bot merged commit 7a322f9 into rust-lang:master Mar 3, 2020

edwin0cheng mentioned this pull request Mar 3, 2020

Tracking issue for builtin declarative macros #2212

Closed

21 tasks

edwin0cheng deleted the new-eager-concat branch March 3, 2020 17:47

lnicola mentioned this pull request Mar 4, 2020

Panic in inlay_hints #3310

Closed

edwin0cheng mentioned this pull request Mar 5, 2020

Completion not working with the gl crate #3467

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement concat eager macro #3392

Implement concat eager macro #3392

edwin0cheng commented Mar 2, 2020 •

edited

Loading

edwin0cheng commented Mar 2, 2020

matklad commented Mar 2, 2020

matklad Mar 3, 2020

edwin0cheng Mar 3, 2020

matklad Mar 3, 2020

edwin0cheng Mar 3, 2020

matklad Mar 3, 2020

edwin0cheng Mar 3, 2020 •

edited

Loading

matklad Mar 3, 2020

edwin0cheng commented Mar 3, 2020

matklad commented Mar 3, 2020

bors bot commented Mar 3, 2020

edwin0cheng commented Mar 3, 2020

bors bot commented Mar 3, 2020

lnicola commented Mar 3, 2020

edwin0cheng commented Mar 3, 2020 •

edited

Loading

Implement concat eager macro #3392

Implement concat eager macro #3392

Conversation

edwin0cheng commented Mar 2, 2020 • edited Loading

edwin0cheng commented Mar 2, 2020

matklad commented Mar 2, 2020

matklad Mar 3, 2020

Choose a reason for hiding this comment

edwin0cheng Mar 3, 2020

Choose a reason for hiding this comment

matklad Mar 3, 2020

Choose a reason for hiding this comment

edwin0cheng Mar 3, 2020

Choose a reason for hiding this comment

matklad Mar 3, 2020

Choose a reason for hiding this comment

edwin0cheng Mar 3, 2020 • edited Loading

Choose a reason for hiding this comment

matklad Mar 3, 2020

Choose a reason for hiding this comment

edwin0cheng commented Mar 3, 2020

matklad commented Mar 3, 2020

bors bot commented Mar 3, 2020

Build failed

edwin0cheng commented Mar 3, 2020

bors bot commented Mar 3, 2020

Build succeeded

lnicola commented Mar 3, 2020

edwin0cheng commented Mar 3, 2020 • edited Loading

edwin0cheng commented Mar 2, 2020 •

edited

Loading

edwin0cheng Mar 3, 2020 •

edited

Loading

edwin0cheng commented Mar 3, 2020 •

edited

Loading