-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Add new syntax extension fourcc!() #9255
Conversation
r? @pcwalton |
let little = match endian { | ||
None => cfg!(target_endian = "little"), | ||
Some(Ident{ident, span}) => { | ||
if ident.name == intern("little") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be cleaner as "little" == cx.str_of(ident)
or even
match cx.str_of(ident).as_slice() {
"little" => true,
"big" => false,
_ => { .. }
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the match
. I'll make that adjustment.
All of the comments have been addressed. r? @alexcrichton or @pcwalton |
|
||
fn target_endian_little(cx: @ExtCtxt, sp: Span) -> bool { | ||
let lit = Spanned{node: ast::lit_str(@"little"), span: sp}; | ||
let meta = @Spanned{node: ast::MetaNameValue(@"target_endian", lit), span: sp}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let meta = cx.meta_name_value(sp, @"target_endian", ast::lit_str(@"little"));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's a lot simpler.
The code itself looks fine to me, but I'm worried about feature creep in general. This is a great example of where dynamically loadable syntax extensions would be awesome, but I'm not sure that we need something like this by default for all crates in the compiler. What do others think about inclusion? |
This would be great for dynamically-loaded, I agree. I was motivated to add it because @pcwalton said yesterday in IRC that a feature like this could be used to speed up the compiler.
although @thestinger did indicate that syntax extensions can't be used in match at the moment. Also as you'll note @pcwalton did suggest something like |
After attempting to implement |
I'm not in favor of putting this special-purpose macro directly in the compiler. Any syntax extensions added now need to be specced and maintained forever. |
Since this is for rustc's use maybe there's some mechanism or convention we can add to discourage this for general use. |
We already need a way to opt in to |
cc #9304 |
Needs a rebase. |
fourcc!() allows you to embed FourCC (or OSType) values that are evaluated as u32 literals. It takes a 4-byte ASCII string and produces the u32 resulting in interpreting those 4 bytes as a u32, using either the platform-native endianness, or explicitly as big or little endian.
With compiler features now in-flight, I'd be more comfortable if this waited for that to land and then rebased on top of it with a feature-gate instead. |
Ooh |
(for the record the build was interrupted, I didn't read the GH comments, was just remembering IRC discussion) |
This implements the necessary logic for gating particular features off by default in the compiler. There are a number of issues which have been wanting this form of mechanism, and this initially gates features which we have open issues for. Additionally, this should unblock #9255
This implements the necessary logic for gating particular features off by default in the compiler. There are a number of issues which have been wanting this form of mechanism, and this initially gates features which we have open issues for. Additionally, this should unblock #9255
Closing due to inactivity, but feel free to reopen if more progress is made! |
I was looking into #9303 and was curious if this would still be valuable. @kballard had already done 99% of the work, so I brought the branch up to date and added a feature gate. Any feedback would be appreciated; I wasn't sure if this should be set up as a syntax extension with `#[macro_registrar]`, and if so, where it should be located. Original PR is here: #9255 TODO: * [x] Convert to loadable syntax extension * [x] Default to big endian * [x] Add `target` identifier * [x] Expand to include code points 128-255
fourcc!() allows you to embed FourCC (or OSType) values that are
evaluated as u32 literals. It takes a 4-byte ASCII string and produces
the u32 resulting in interpreting those 4 bytes as a u32, using either
the platform-natiev endianness, or explicitly as big or little endian.