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

std: Stablize the macros module #20657

Merged
merged 1 commit into from
Jan 8, 2015
Merged

Conversation

alexcrichton
Copy link
Member

This commit performs a pass over the std::macros module, applying stability
attributes where necessary. In particular, this audits macros for patterns such
as:

  • Standard use of forward-to-format-args via $($arg:tt)* (or +)
  • Prevent macro-defined identifiers from leaking into expression arguments as
    hygiene is not perfectly implemented.
  • Wherever possible, $crate is used now.

Specifically, the following actions were taken:

  • The std::macros module itself is no longer public.
  • The panic! macro is stable
  • The assert! macro is stable
  • The assert_eq! macro is stable
  • The debug_assert! macro is stable
  • The debug_assert_eq! macro is stable
  • The unreachable! macro is stable after removing the extra forms to bring the
    definition in line with the unimplemented! macro.
  • The try! macro is stable
  • The vec! macro is stable

@alexcrichton
Copy link
Member Author

r? @aturon

@rust-highfive rust-highfive assigned pcwalton and aturon and unassigned pcwalton Jan 6, 2015
@rust-highfive
Copy link
Collaborator

r? @pcwalton

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

($fmt:expr, $($arg:tt)*) => ({
panic!(concat!("internal error: entered unreachable code: ", $fmt), $($arg)*)
});
() => (panic!("internal error: entered unreachable code"))
Copy link
Member

Choose a reason for hiding this comment

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

What's the rationale for dropping the functionality here, rather than beefing up unimplemented?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd personally like to remove both of them as they're just longer versions of panic! in my mind, but failing that I'd rather stick to a more conservative approach where we don't just duplicate-panic-in-each-macro just yet.

@aturon
Copy link
Member

aturon commented Jan 6, 2015

This basically looks good to me, but we may want to get input from someone with deeper macros knowledge.

Also, are we 100% sure we want to stabilize vec!? It's probably so widely-used that we can't ever get rid of it...

@alexcrichton
Copy link
Member Author

With regard to vec!, you're thinking of perhaps a more generic collections-creating macro? The current implementation is pretty super-specialized to Vec (deconstructing Box<[T]> and reassembling), but it is unfortunate that we don't have macros for other containers. Furthermore, it'd be unfortunate to have either:

  • A generic macro for all containers, plus a more efficient vec! macro
  • A macro-per-container

I do agree though that we've put ourselves in a corner with vec! right now, we don't really have a great way out of it :(

cc @kmcallister, @sfackler

@@ -353,9 +350,6 @@ fn initial_syntax_expander_table(ecfg: &expand::ExpansionConfig) -> SyntaxEnv {
syntax_expanders.insert(intern("option_env"),
builtin_normal_expander(
ext::env::expand_option_env));
syntax_expanders.insert(intern("bytes"),
Copy link
Member

Choose a reason for hiding this comment

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

If this is being removed, shouldn't the ext::bytes module be removed as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Aha, indeed!

@sfackler
Copy link
Member

sfackler commented Jan 6, 2015

The commit should be tagged as a [breaking-change] since fmt!, include_bin! and bytes! are going away.

@aturon
Copy link
Member

aturon commented Jan 6, 2015

@alexcrichton Yep, those were pretty much exactly my thoughts. The only question is whether we want to fully commit now, or wait till beta to admit that we have to.

@kmcallister
Copy link
Contributor

I think the best case outcome for vec! is a generic macro that uses traits to work for multiple kinds of containers, at which point vec! could become a synonym.

@kmcallister
Copy link
Contributor

These changes look good to me.

@alexcrichton
Copy link
Member Author

rebased, re-r? @aturon

This commit performs a pass over the `std::macros` module, applying stability
attributes where necessary. In particular, this audits macros for patterns such
as:

* Standard use of forward-to-format-args via `$($arg:tt)*` (or `+`)
* Prevent macro-defined identifiers from leaking into expression arguments as
  hygiene is not perfectly implemented.
* Wherever possible, `$crate` is used now.

Specifically, the following actions were taken:

* The `std::macros` module itself is no longer public.
* The `panic!` macro is stable
* The `assert!` macro is stable
* The `assert_eq!` macro is stable
* The `debug_assert!` macro is stable
* The `debug_assert_eq!` macro is stable
* The `unreachable!` macro is stable after removing the extra forms to bring the
  definition in line with the `unimplemented!` macro.
* The `try!` macro is stable
* The `vec!` macro is stable

[breaking-change]
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 8, 2015
@bors bors merged commit 209c701 into rust-lang:master Jan 8, 2015
@alexcrichton alexcrichton deleted the stabilize-macros branch January 20, 2015 06:46
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.

7 participants