Rewrite core::int, etc. to use item macros. Remove #[merge] hack

[brson]

The core integer modules are all built from the same template files using a parser hack to merge in the type-specific portions. Now that we have item macros these could be expressed with macro_rules! int_template { ... }. Remove the #[merge] attribute.

[isaacaggrey]

I'm interested in taking this on as my first Rust bug, but I need to mess around with modules and item macros a bit more to understand this specific scenario.

[brson]

Good luck! I myself am not too familiar with macros, but I have faith that it will work.

[isaacaggrey]

Re implementation: I can't seem to think of a solution without cluttering core.rc. It's straightforward enough to use an item macro that takes in an item for each type-specific chunk to combine with the generic code, but it seems necessary to make these declarations in core.rc rather than declaring the modules in external files as it is now.

Is this expected or is there a different approach here that I'm missing?

[brson]

@isaacaggrey Here's the pattern the compiler uses for importing macros: https://github.com/mozilla/rust/blob/incoming/src/librustc/middle/typeck/infer/lub.rs#L20

Does that work for you?

[brson]

Oh, you said it's the type-specific parts that are going to cause clutter. How bad is it?

[brson]

The type-specific bits are mostly pretty small. int and uint have some extra crud in them that should probably be integrated into all the integer modules, but it looks like, ideally, the only parameters need to be the name of the type and the number of bits.

I don't have a great solution for putting that stuff in another file, although we could have some variant of include! that will insert multiple items (or maybe it already works):

// core.rc

!include("int_modules.rs")

Could also use reexports:

priv mod integer_modules;

pub use integer_modules::*;

That solution would not work well with rustdoc - not sure what it would do, but it wouldn't document the modules as core::int, etc. like we need. Rustdoc needs to handle this scenario anyway, so it still may be acceptable here.

[isaacaggrey]

I committed an XY Problem.

What I meant about clutter was the naive solution of moving int-template.rs into core.rc as a macro and declaring each type module within core.rc since I didn't know it was possible to import macros (which is what I really wanted to do).

In any case, the include! pattern is exactly the push I needed! Thanks, @brson!

[isaacaggrey]

I had a working patch on master, but upon grabbing incoming I hit resolve errors that I assume are due to the crate-relative use change in tandem with using the workaround for macros expanding to >1 item (issue #3086 comment #4).

Implementation

// core.rc

...

// FIXME(#3114): Macro import/export.
fn macros() { include!("integer-templates.rs"); }

/* Primitive types */

// pub mod i8 { int_template!(i8, 8) } // what it'll look like w/o temporary workaround below

pub mod i8  {
    int_template!(i8, 8) // accepts type and number of bits
    pub use i8::inst::*;
}
...

// integer-templates.rs
{

macro_rules! int_template (

  ($typ:ty, $bits:expr) => {

    pub mod inst { // FIXME(#3086): Macro expansion to multiple items

      use char;
      use cmp;
      use cmp::{Eq, Ord};
      use from_str::FromStr;
      use iter;
      use num;
      use num::Num::from_int;
      use str;
      use uint;
      use vec;

      pub type T = $typ;
      pub const bits  : uint = $bits;
      pub const bytes : uint = (bits / 8);

      // rest of int-template.rs code
    }
  }
)

macro_rules! uint_template (
  // etc
)

}

I re-export at the end of each module to work around the workaround for multiple item expansion (extra module mod inst full of those items). The int_template! macro is simply a wrapper around the existing int-template.rs file and a merge of the other signed integer type files since there's nothing type specific. Similarly, there's a uint_template! macro doing the same for uint-template.rs and unsigned integer type files (except u8's is_ascii).

Aside: Another macro could be created to encapsulate what's common to both macros, but I'm not sure if that helps or hurts readability (yo dawg, macro in a macro...).

Issues

The compiler complains about not resolving the additional use declarations added in #4174 (use char;, use cmp;, etc). Interestingly enough, once those declarations are removed everything works...for the first pass of the compiler (stage0), but fails at stage1 with resolve errors for the removed use declarations.

I tried placing the use declarations with the module body in core.rc, and I got a different set of resolve errors (e.g., ::i8::to_str).

I imagine there might be some #[cfg] magic that could be used, but I'm not familiar with the stages of the compiler.

How can I modify the use declarations and/or re-export correctly to solve these issues?

(Happy New Year and sorry for the length; feel free to refer me to someone else if necessary!)

[catamorphism]

Not critical for 0.6 -- de-milestoning

[alexcrichton]

This should be more than possible after the next snapshot. Currently the stage0 compiler expands

macro_rules! a(
  () => (pub mod a {
    #[cfg(a)] pub fn foo() {}
    #[cfg(b)] pub fn foo() {}
  })
)

a!()

fn main() { a::foo(); }

to the a module having both definitions of foo (regardless of --cfg flags passed to rustc). This is fixed within the compiler, however (expansion happens much earlier in the compilation process). It's possible to do now, but it would be much less ugly after the next snapshot.

This is a problem because a few methods in the templates (the iterators) are cfg'd for different stages.

[isaacaggrey]

@alexcrichton Is it not the case that issues #4375 and #3114 are still blocking this issue from being fixed in a clean way? I don't think cfg attributes help here, but I very well may be misunderstanding your example.

[alexcrichton]

Well in the ideal world both those issues can be resolved, but I think that the main purpose of this bug is to remove the #[merge] hack which seems like more of an issue to me. With that in mind, macros can definitely used to implement these modules.

I started working on this before I realized it was already taken. You can use the #[macro_escape] attribute to "export" macros, so 3114 isn't completely necessary. Also, 4375 would be nice, but you can also just have a private module and then in the actual module you just put pub use generated::* or something like that. The unfortunate part is that rustdoc becomes pretty useless, but there's open bugs for that as far as I know as well.

I might take a stab at 4375 in the meantime, but I'll let you take over from here!

[isaacaggrey]

@alexcrichton If you already have working code that fixes this issue, then by all means submit the pull request! I'm not self-centered enough to block working code from improving a project I'm interested in. :-) I'll just have to find a different first bug to work on.

In hindsight, I should have requested that brson assign the issue to me since I suppose the comment I made or the work I attempted to show is not "official".

You have my blessing.

brson could you please assign this bug to @alexcrichton?

[alexcrichton]

Oh my code isn't working at all, I stopped once I hit the cfg bug with macros, so I'm gonna stick around until the next snapshot anyway.

[isaacaggrey]

Oh, I see. You're further along than I am, since my version of the code is back when the crate-relative use change caused issues.

Also, I didn't see your last line in your other comment, thank you. In that case, I'll ask brson to assign me to the bug in order to avoid further confusion. :-)

@brson could you assign me to this bug?

[jdm]

github does not allow assigning bugs to users who are not formally part of the project's team, unfortunately.

[isaacaggrey]

Ah, I didn't know that! Well, I'm counting on submitting a pull request with working code soon to avoid further confusion.