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

Fix syntax extensions for #![no_std] #21912

Closed
wants to merge 2 commits into from

Conversation

kmcallister
Copy link
Contributor

This fixes derive (for most traits, c.f. #21880), format!, and for loops. Slice syntax is still broken because the desugar happens in the parser. I'll follow up on that bug (#21827).

This duplicates the functionality of std::fmt::format as String::format, which works whether libstd or just libcollections is available. fmt::format is marked as stable but that's not a hard requirement until 1.0-final. So I'd like to stabilize String::format and deprecate fmt::format to be removed before 1.0. I can prepare an RFC for that if necessary. There's no visible change for users of the format! macro, except that it works without libstd.

This is based on my old PR #17100.

r? @alexcrichton

cc @huonw, @sfackler

@alexcrichton
Copy link
Member

I think we may want to hold off on this until the outcome of #21833 is decided. This has some surprising semantics if #![no_std] is used (forces linking to unstable crates). If #![no_std] itself is feature gated though, then I think this is fine.

let mut output = String::new();
let _ = write!(&mut output, "{}", args);
output
}
Copy link
Member

Choose a reason for hiding this comment

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

This should avoid adding new APIs to String for now, if necessary then it's likely that the entire std::fmt module can live in libcollections and this function need not be added just yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think String::format is a better, more descriptive name. Is there a specific reason we can't revisit this API choice?

Copy link
Member

Choose a reason for hiding this comment

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

It's fine to revisit, but that change requires an RFC (due to String being in the prelude).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll prepare one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to back out this change to now to wait for that resolution?

Copy link
Member

Choose a reason for hiding this comment

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

Er, back out this change to land the rest of this PR ahead of the RFC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I can do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think String::format is a better, more descriptive name.
On Feb 4, 2015 6:46 AM, "Keegan McAllister" notifications@github.com
wrote:

In src/libcollections/string.rs
#21912 (comment):

  • ///
  • /// # Example
  • ///
  • /// ```rust
  • /// let s = String::format(format_args!("Hello, {}!", "world"));
  • /// assert_eq!(s, "Hello, world!".to_string());
  • /// ```
  • #[unstable(feature = "collections",
  •           reason = "duplicates std::fmt::format, only needed when libstd is missing")]
    
  • pub fn format(args: fmt::Arguments) -> String {
  •    // FIXME #21826
    
  •    use core::fmt::Writer;
    
  •    let mut output = String::new();
    
  •    let _ = write!(&mut output, "{}", args);
    
  •    output
    
  • }

I think String::format is a better, more descriptive name. Is there a
specific reason we can't revisit this API choice?


Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rust/pull/21912/files#r24047873.

@kmcallister
Copy link
Contributor Author

This has some surprising semantics if #![no_std] is used (forces linking to unstable crates).

If you're using no_std then you probably have extern crate core already. Certainly it's reasonable to expect it if you want to derive traits from core. Likewise, if you're calling format! and expecting a String result then it's reasonable to expect that you have #[macro_use] extern crate collections -- in fact that's how you'd get the format! macro.

In no case does the syntax extension insert an extern crate for you.

I agree that no_std is very hard to use without unstable crates, which is why I'd like to feature-gate it. But I don't think we change much by making syntax extensions work when you do have those unstable crates.

@kmcallister
Copy link
Contributor Author

Rebased, and changed to the collections::fmt::format approach that doesn't alter stable APIs.

@kmcallister
Copy link
Contributor Author

fwiw, gating #![no_std] was just approved as P-high, 1.0 beta.

@kmcallister
Copy link
Contributor Author

Folding this into #21988.

@kmcallister kmcallister closed this Feb 6, 2015
bors added a commit that referenced this pull request Feb 8, 2015
Fixes #21833.

[breaking-change]

r? @alexcrichton 

The tests in #21912 will also need `#[feature(no_std)]`. If you're okay with both PRs, I can merge and test them.
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.

None yet

3 participants