Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify format and document related macros #9153

Merged
merged 3 commits into from
Sep 15, 2013

Conversation

alexcrichton
Copy link
Member

This follows from the discussion in #9012.

  • All macros are now defined in terms of format_args! allowing for removal of a good bit of code in the syntax extension
  • The syntax extension is now in a more aptly-named file, format.rs
  • Documentation was added for the format!-related macros.

@@ -74,4 +71,11 @@ fn main() {

format!("foo } bar"); //~ ERROR: unmatched `}` found
format!("foo }"); //~ ERROR: unmatched `}` found

// FIXME(#7970) the spans on these errors are pretty terrible, some come
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the sufficiently bad that the expansion should happen in ext::format? i.e. have all the separate expand_format, expand_write etc, with the implementation (with a macro to avoid mega-repetition, presumably):

fn expand_format(args...) {
    let expr = <expr for `::std::fmt::rt::format`>;

    expand_ifmt(Some(expr), args...);
}
// ... expand_write etc.

fn expand_ifmt(call_func: Option<@expr>, ...) {
   // if `call_func` is None parse a function as the first arg,
   // otherwise use `call_func` for the final `expr_call` in `to_expr`
}

The FIXME could then go there.

If this is going to replace fmt! then we should make it properly useable, and, frankly, the errors from inside macro expansion are pretty much useless: no information about the original invocation site at all. (Is #5794 a more appropriate issue for this FIXME?)

@alexcrichton
Copy link
Member Author

@huonw and I think that this is blocked in its current state on #5794. I'll look into fixing that soon, otherwise I think that this perhaps just shouldn't be accepted because it makes certain kinds of errors 10x worse.

This renames the syntax-extension file to format from ifmt, and it also reduces
the amount of complexity inside by defining all other macros in terms of
format_args!
The same fix as before is still relevant, I just forgot to update the
expand_stmt macro expansion site. The tests for format!() suffice as tests for
this change.
bors added a commit that referenced this pull request Sep 15, 2013
This follows from the discussion in #9012.

* All macros are now defined in terms of `format_args!` allowing for removal of a good bit of code in the syntax extension
* The syntax extension is now in a more aptly-named file, `format.rs`
* Documentation was added for the `format!`-related macros.
@bors bors closed this Sep 15, 2013
@bors bors merged commit 6406138 into rust-lang:master Sep 15, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants