Ergonomic format_args! #33642

xen0n · 2016-05-14T19:10:57Z

Fixes #9456 (at last).

Not a ground-up rewrite of the existing machinery, but more like an added intermediary layer between macro arguments and format placeholders. This is now implementing Rust RFC 1618!

rust-highfive · 2016-05-14T19:11:06Z

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

xen0n · 2016-05-15T08:15:14Z

This caused some small regressions here and there. I'll be slowly fixing them; compilation time is very long.

Non-regressions, just TODO:

compile-fail/ifmt-bad-arg.rs
run-pass/ifmt.rs

Noticed regressions:

compile-fail/macro-backtrace-println.rs
doctest fmt_3 ({:.*} didn't eat the precision argument)

xen0n · 2016-05-16T06:17:10Z

Implemented correct implicit args handling by resolving all CountIsNextParams and ArgumentNexts into absolute references during parse. Which means, for example, {foo:.*} {} {1:.*} {:.*} {bar:.*} now parses exactly like {foo:.0$} {1} {1:.2$} {4:.3$} {bar:.5$}. Which obviates the need to handle any next-references during expansion and outputting; will remove these later, after I finish up an RFC for this PR later this week.

EDIT: Whoops, CountIsParam(i) should be CountIsNextParam. CountIsParam(i) is also touched but it's not related to resolution (it's expansion).

…tsakis syntax_ext: format: nest_level's are no more Just noticed this while working on rust-lang#33642 and here's a quick fix, shouldn't touch anything else. It's some historic code indeed...

aturon · 2016-05-17T18:57:27Z

r? @alexcrichton

…tsakis syntax_ext: format: nest_level's are no more Just noticed this while working on rust-lang#33642 and here's a quick fix, shouldn't touch anything else. It's some historic code indeed...

alexcrichton · 2016-05-20T18:14:33Z

Thanks for the PR! I'll take a closer look at this once rust-lang/rfcs#1618 is handled

xen0n · 2016-05-21T15:22:28Z

Fixed test cases broken by this PR, and added new cases as well.

Currently make check fails on several unrelated tests:

run-pass/issue-28950.rs
run-pass/panic-runtime/lto-abort.rs
run-pass/panic-runtime/lto-unwind.rs
run-pass/sepcomp-lib-lto.rs

I'm not sure why those keep popping up, it wasn't the case several days ago. Testing master branch right now, will edit this once finished.

Update: master fails on those tests too. Pretty sure these aren't my fault. But again it may be my environment... I have to sleep now though, it's 3 am, too tired to launch yet another investigation.

Another update: I recompiled without --enable-debug and the failures were gone. Surely there is some nasty interaction between LTO and debug code generation. But at least I could sanely continue my work. 😞

xen0n · 2016-05-22T05:47:28Z

Travis test completed without the unrelated failures, seems there is a pretty-print round-trip failure:

--- pretty.expected.rs  2016-05-22 13:39:14.080831211 +0800
+++ pretty.actual.rs    2016-05-22 13:38:56.912831929 +0800
@@ -178,7 +178,6 @@
        , b = "hijklmn" , c = 3 ) , "abcd hijk 4\nabc hij 3");
     t!(format ! ( "{a:.*} {0} {:.*}" , 4 , 3 , "efgh" , a = "abcdef" ) ,
        "abcd 4 efg");
-
     // Test that pointers don't get truncated.
     {
         let val = usize::MAX;

Apart from this and deduplication of argument objects, the feature feels almost ready. Waiting for RFC resolution 😄

alexcrichton · 2016-05-23T15:24:02Z

Ah yeah unfortunately we've had bad luck with debuginfo and LTO in the past (lots of weird LLVM assertions), but thanks for the progress update!

xen0n · 2016-06-01T14:28:27Z

I immediately gave up fixing the pretty print issue after trying to produce a minimal test case and got this:

macro_rules! t {
    ($a:expr, $b:expr) => { assert_eq!($a, $b) }
}

pub fn main() {
    t!(format!("{:x}{:X}{:x}{:X}{:x}{a:X}",1,2,3,4,5,a=15), "abcdefghijkl");




    // every time this code gets pretty-printed, a newline is eaten!
}

Might as well reduce the length of affected lines, as the bug seems to only occur when a line wrap is needed in the output. I'll finish the argument object de-duplication and then we can start reviewing.

xen0n · 2016-06-02T14:11:47Z

Rebased and cleaned up a little, should be ready for review!

PS: The reason I think an RFC is really needed, is that some serious documentation changes and maybe deprecation inside the formatting runtime will have to be done. However as the RFC is already submitted and waiting through its FCP this point is already moot.

alexcrichton · 2016-06-05T07:43:13Z

src/test/run-pass/ifmt.rs

+    // named arg only used as count
+    t!(format!("{0:.a$}", "aaaaaa", a=2), "aa");
+
+


Can you add a test here as well with side effects on the evaluated expression to ensure it's only calculated once even though it may be formatted multiple times.

bors · 2016-07-04T12:07:03Z

☔ The latest upstream changes (presumably #34638) made this pull request unmergeable. Please resolve the merge conflicts.

xen0n · 2016-07-04T13:53:49Z

Rebased again and (finally found time to do so!) added comments to facilitate better understanding. Hope this is not too late for 1.10!

alexcrichton · 2016-07-13T18:22:42Z

Oops, sorry this fell off the radar! I think this got lost in my inbox by accident...

Ok, everything looks good to me, thanks @xen0n! Could you also squash the commits down? Other than that r=me

format: beautifully get rid of ArgumentNext and CountIsNextParam Now that CountIsNextParam and ArgumentNext are resolved during parse, the need for handling them outside of libfmt_macros is obviated. Note: *one* instance of implicit reference handling still remains, and that's for implementing `all_args_simple`. It's trivial enough though, so in this case it may be tolerable.

@alexcrichton

Converts named argument references into indices, right after verification as suggested by @alexcrichton. This drastically simplifies the whole process!

This commit removed the restriction of only allowing one type per argument. This is achieved by adding mappings between macro arguments and format placeholders, then taking the mapping into consideration when emitting the Arguments expression. syntax_ext: format: fix implicit positional arguments syntax_ext: format: don't panic if no args given for implicit positional args Check the list lengths before use. Fixes regression of `compile-fail/macro-backtrace-println.rs`. syntax_ext: format: also map CountIsParam indices to expanded args syntax_ext: format: fix ICE in case of malformed format args

format: workaround pretty-printer to pass tests

xen0n · 2016-07-13T19:15:28Z

Squashed all review-generated commits into respective commits and rebased again. Also clarified a few _ => panic! cases for better maintainability (no worries if some enum variants got added someday). Should be good to go now! @alexcrichton

(Edit: As for missing 1.10, I was also taking a break from Rust recently so I don't really mind. 😈 )

alexcrichton · 2016-07-13T20:08:45Z

@bors: r+ 51e54e5

Thanks again!

bors · 2016-07-13T20:26:58Z

⌛ Testing commit 51e54e5 with merge db71987...

Ergonomic format_args! Fixes #9456 (at last). Not a ground-up rewrite of the existing machinery, but more like an added intermediary layer between macro arguments and format placeholders. This is now implementing Rust RFC 1618!

bors · 2016-07-13T23:27:28Z

…crichton Update std::fmt module docs for landing of rust-lang#33642. Since rust-lang#33642, it's no longer true that all references to a given format argument must use the same type. The docs don't seem to have been updated.

rust-highfive assigned aturon May 14, 2016

xen0n force-pushed the ergonomic-format-macro branch from adf0be0 to ca2c003 Compare May 16, 2016 06:31

xen0n mentioned this pull request May 16, 2016

syntax_ext: format: nest_level's are no more #33666

Merged

xen0n mentioned this pull request May 17, 2016

Ergonomic format_args! rust-lang/rfcs#1618

Merged

rust-highfive assigned alexcrichton and unassigned aturon May 17, 2016

xen0n force-pushed the ergonomic-format-macro branch 3 times, most recently from 93b7b3f to 9614a17 Compare May 21, 2016 15:13

xen0n changed the title ~~[WIP] [DO NOT MERGE] Ergonomic format macro~~ [WIP] Ergonomic format_args! May 21, 2016

xen0n force-pushed the ergonomic-format-macro branch 2 times, most recently from b96da3d to 68dd135 Compare June 2, 2016 14:01

xen0n changed the title ~~[WIP] Ergonomic format_args!~~ Ergonomic format_args! Jun 2, 2016

xen0n force-pushed the ergonomic-format-macro branch 2 times, most recently from dea6bfd to 7037411 Compare June 3, 2016 14:10

alexcrichton reviewed Jun 5, 2016
View reviewed changes

xen0n force-pushed the ergonomic-format-macro branch from 77c4bf1 to 6d819f4 Compare July 4, 2016 12:50

alexcrichton added the relnotes Marks issues that should be documented in the release notes of the next release. label Jul 5, 2016

xen0n added 10 commits July 14, 2016 02:44

libfmt_macros: resolve all implicit refs while parsing

71949f3

syntax_ext: format: separate verification and translation of pieces

0e2a963

syntax_ext: format: resolve named arguments early

1ace7f0

Converts named argument references into indices, right after verification as suggested by @alexcrichton. This drastically simplifies the whole process!

syntax_ext: format: rename variants of ArgumentType for clarity

eb5417b

syntax_ext: format: process counts uniquely and separately

f457e6c

syntax_ext: format: de-duplicate argument objects

7180994

format: add tests for ergonomic format_args!

03563b1

format: workaround pretty-printer to pass tests

syntax_ext: format: better code documentation

51e54e5

xen0n force-pushed the ergonomic-format-macro branch from 466558e to 51e54e5 Compare July 13, 2016 19:11

bors merged commit 51e54e5 into rust-lang:master Jul 13, 2016

xen0n deleted the ergonomic-format-macro branch July 14, 2016 04:29

TimNN mentioned this pull request Jul 28, 2016

ICE: 'index out of bounds: the len is 1 but the index is 1' for println! and writeln! #35082

Closed

jimblandy added a commit to jimblandy/rust that referenced this pull request Feb 23, 2017

Update std::fmt module docs for landing of rust-lang#33642.

6b8e175

jimblandy mentioned this pull request Feb 23, 2017

Update std::fmt module docs for landing of #33642. #40048

Merged

sinkuu mentioned this pull request Oct 13, 2017

Assignment inside format_args! is ignored #45256

Closed

Amanieu mentioned this pull request Feb 12, 2020

format! allows named arguments to be referenced by position #69110

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ergonomic format_args! #33642

Ergonomic format_args! #33642

xen0n commented May 14, 2016 •

edited

rust-highfive commented May 14, 2016

xen0n commented May 15, 2016 •

edited by vadimcn

xen0n commented May 16, 2016 •

edited

aturon commented May 17, 2016

alexcrichton commented May 20, 2016

xen0n commented May 21, 2016 •

edited

xen0n commented May 22, 2016

alexcrichton commented May 23, 2016

xen0n commented Jun 1, 2016 •

edited

xen0n commented Jun 2, 2016

alexcrichton Jun 5, 2016

bors commented Jul 4, 2016

xen0n commented Jul 4, 2016

alexcrichton commented Jul 13, 2016

xen0n commented Jul 13, 2016 •

edited

alexcrichton commented Jul 13, 2016

bors commented Jul 13, 2016

bors commented Jul 13, 2016

		// named arg only used as count
		t!(format!("{0:.a$}", "aaaaaa", a=2), "aa");

Ergonomic format_args! #33642

Ergonomic format_args! #33642

Conversation

xen0n commented May 14, 2016 • edited

rust-highfive commented May 14, 2016

xen0n commented May 15, 2016 • edited by vadimcn

xen0n commented May 16, 2016 • edited

aturon commented May 17, 2016

alexcrichton commented May 20, 2016

xen0n commented May 21, 2016 • edited

xen0n commented May 22, 2016

alexcrichton commented May 23, 2016

xen0n commented Jun 1, 2016 • edited

xen0n commented Jun 2, 2016

alexcrichton Jun 5, 2016

Choose a reason for hiding this comment

bors commented Jul 4, 2016

xen0n commented Jul 4, 2016

alexcrichton commented Jul 13, 2016

xen0n commented Jul 13, 2016 • edited

alexcrichton commented Jul 13, 2016

bors commented Jul 13, 2016

bors commented Jul 13, 2016

xen0n commented May 14, 2016 •

edited

xen0n commented May 15, 2016 •

edited by vadimcn

xen0n commented May 16, 2016 •

edited

xen0n commented May 21, 2016 •

edited

xen0n commented Jun 1, 2016 •

edited

xen0n commented Jul 13, 2016 •

edited