Skip to content

Commit 89002e7

Browse files
committed
ergonomic-format-args: address review comments
* feature name seems not applicable as this is not something to be gated * updated to reflect latest implementation * removed most details for higher-level overview * reworded slightly
1 parent 4b02fa0 commit 89002e7

File tree

1 file changed

+34
-74
lines changed

1 file changed

+34
-74
lines changed

text/0000-ergonomic-format-args.md

+34-74
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
- Feature Name: `ergonomic_format_args`
1+
- Feature Name: (not applicable)
22
- Start Date: 2016-05-17
33
- RFC PR: (leave this empty)
44
- Rust Issue: (leave this empty)
@@ -7,6 +7,9 @@
77
[summary]: #summary
88

99
Removes the one-type-only restriction on `format_args!` arguments.
10+
Expressions like `format_args!("{0:x} {0:o}", foo)` now work as intended,
11+
where each argument is still evaluated only once, in order of appearance
12+
(i.e. left-to-right).
1013

1114
# Motivation
1215
[motivation]: #motivation
@@ -29,8 +32,6 @@ so the mechanism as a whole certainly needs more love.
2932
# Detailed design
3033
[design]: #detailed-design
3134

32-
## Overview
33-
3435
Formatting is done during both compile-time (expansion-time to be pedantic)
3536
and runtime in Rust. As we are concerned with format string parsing, not
3637
outputting, this RFC only touches the compile-time side of the existing
@@ -39,17 +40,22 @@ formatting mechanism which is `libsyntax_ext` and `libfmt_macros`.
3940
Before continuing with the details, it is worth noting that the core flow of
4041
current Rust formatting is *mapping arguments to placeholders to format specs*.
4142
For clarity, we distinguish among *placeholders*, *macro arguments* and
42-
*generated `ArgumentV1` objects*. They are all *italicized* to provide some
43+
*argument objects*. They are all *italicized* to provide some
4344
visual hint for distinction.
4445

45-
To implement the proposed design, first we resolve all implicit references to
46-
the next argument (*next-references* for short) during parse; then we modify
47-
the macro expansion to make use of the now explicit argument references,
48-
preserving the mapping.
46+
To implement the proposed design, the following changes in behavior are made:
47+
48+
* implicit references are resolved during parse of format string;
49+
* named *macro arguments* are resolved into positional ones;
50+
* placeholder types are remembered and de-duplicated for each *macro argument*,
51+
* the *argument objects* are emitted with information gathered in steps above.
4952

50-
## Parse-time next-reference resolution
53+
As most of the details is best described in the code itself, we only
54+
illustrate some of the high-level changes below.
5155

52-
Currently two forms of next-references exist: `ArgumentNext` and
56+
## Implicit reference resolution
57+
58+
Currently two forms of implicit references exist: `ArgumentNext` and
5359
`CountIsNextParam`. Both take a positional *macro argument* and advance the
5460
same internal pointer, but format is parsed before position, as shown in
5561
format strings like `"{foo:.*} {} {:.*}"` which is in every way equivalent to
@@ -58,68 +64,23 @@ format strings like `"{foo:.*} {} {:.*}"` which is in every way equivalent to
5864
As the rule is already known even at compile-time, and does not require the
5965
whole format string to be known beforehand, the resolution can happen just
6066
inside the parser after a *placeholder* is successfully parsed. As a natural
61-
consequence, both forms of next-reference can be removed from the rest of the
62-
compiler, simplifying work later.
63-
64-
## Expansion-time argument mapping
65-
66-
There are two kinds of *macro arguments*, positional and named. Because of the
67-
apparent type difference, two maps are needed to track *placeholder* types
68-
(known as `ArgumentType`s in the code). In the current implementation,
69-
`Vec<Option<ArgumentType>>` is for positional *macro arguments* and
70-
`HashMap<String, ArgumentType>` is for named *macro arguments*, apparently
71-
neither of which supports multiple types for one *macro argument*. Also, for
72-
constructing the `__STATIC_FMTARGS` we need to first figure out the order for
73-
every *placeholder* in the list of *generated `ArgumentV1` objects*. So we
74-
first classify *placeholders* according to their associated *macro arguments*,
75-
which are all explicit now, then assign each of them a correct index.
76-
77-
### Placeholder type collection
78-
79-
In the proposed design, lists of `ArgumentType`s are used to store
80-
*placeholder* types for each *macro argument* in order. During verification
81-
the *placeholder* type seen for a *macro argument* is simply pushed into the
82-
respective list. This does not remove the ability to sense unused
83-
*macro arguments*, as the list would simply be empty when checked later, just
84-
as it would be `None` in the old `Option<ArgumentType>` version.
85-
86-
### Mapping construction
87-
88-
For consistency with the current implementation, named *macro arguments* are
89-
still put at the end of *generated `ArgumentV1` objects*. Which means we have
90-
to consume all of format string in order to know how many *placeholders* there
91-
are referencing to positional *macro arguments*. As such, the verification
92-
and translation of pieces are now separated with mapping construction in
93-
between.
94-
95-
Obviously, the orders used during mapping and actual expansion must agree, but
96-
fortunately the rules are very simple now only explicit references remain.
97-
We iterate over the list of known positional *macro arguments*, recording the
98-
index at which every bunch of *generated `ArgumentV1` objects* would begin for
99-
each positional *macro argument*. After that, we also record the total number
100-
for mapping the named *macro arguments*, as the relative offsets of named
101-
*placeholders* are already recorded during verification.
102-
103-
### Expansion
104-
105-
With mapping between *placeholders* and *generated `ArgumentV1` objects*
106-
ready at hand, it is easy to emit correct `Argument`s. Scratch space is
107-
provided to `trans_piece` for remembering how many *placeholders* for a given
108-
*macro argument* have been processed. This information is then used to rewrite
109-
all references from using *macro argument* indices to
110-
*generated `ArgumentV1` object* indices, namely:
111-
112-
* `ArgumentIs(i)`
113-
* `ArgumentNamed(n)`
114-
* `CountIsParam(i)`
115-
* `CountIsName(n)`
116-
117-
For the count references, some may suggest that they are now potentially
118-
ambiguous. However considering the implementation of `verify_count`, the
119-
parameter used by each `Count` is individually injected into the list of
120-
*generated `ArgumentV1` objects* as if it were explicitly specified. Also it
121-
is *macro arguments* to be referenced, not the potentially multiple
122-
*placeholders*, so there are in fact no ambiguities.
67+
consequence, both forms can be removed from the rest of the compiler,
68+
simplifying work later.
69+
70+
## Named argument resolution
71+
72+
Not seen elsewhere in Rust, named arguments in format macros are best seen as
73+
syntactic sugar, and we'd better actually treat them as such. Just after
74+
successfully parsing the *macro arguments*, we immediately rewrite every name
75+
to its respective position in the argument list, which again simplifies the
76+
process.
77+
78+
## Processing and expansion
79+
80+
We only have absolute positional references to *macro arguments* at this point,
81+
and it's straightforward to remember all unique *placeholders* encountered for
82+
each. The unique *placeholders* are emitted into *argument objects* in order,
83+
to preserve evaluation order, but no difference in behavior otherwise.
12384

12485
# Drawbacks
12586
[drawbacks]: #drawbacks
@@ -182,5 +143,4 @@ ergonomics is simply bad and the code becomes unnecessarily convoluted.
182143
# Unresolved questions
183144
[unresolved]: #unresolved-questions
184145

185-
* Does the *generated `ArgumentV1` objects* need deduplication?
186-
* Will it break the ABI if handling of next-references in `libcore/fmt` is removed as well?
146+
None.

0 commit comments

Comments
 (0)