You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add hashmap, hashset, treemap, and treeset macros #14726
I wanted to create an issue first asking about this before submitting a pull request.
Can I go ahead an implement hashmap!(), hashset!(), treemap!(), and treeset!() macros for constructing those collections with the given arguments? The syntax would be:
let h = hashmap!("foo" => "bar");
let s = hashset!("foo");
I already have these macros implemented in my own projects, so I'd just have to add them to macros.rs.
If I can add these, is there a process for testing macros? Or would I just replace all occurrences of hash{map,set} and tree{map,set} creation in the tests by the macros?
Reacted by zac garby, Patrick Hayes, Felix Schütt, Benjamin Levy, Robin Brown, Krish, tfkls, Wonhyuk Choi, Anselme and Brian Wo
One problem that I can see off the top of my head is that macros.rs is in the std crate, while all the collections that you are using are in the collections crate. I'm not sure if you could add macros to the collections crate.
I am strongly in favor of getting these macros added somewhere simply because of how common the collections are used. We need a shortcut.
In any case, I think this is a change that requires an RFC. I would prefer having a generic macro (or two of them) that allows constructing any container type, rather than having to manually create one for each type and forcing other data structure implementers to do the same.
@huonw Would that macro work? I tried to write a similar one for the MutableMap trait which wouldn't type check because the type of _thing must be known at the insert:
#![feature(macro_rules)]externcrate collections;macro_rules! map {($($k: expr => $v: expr),*) => {{use std::default::Default;letmut _thing = Default::default();
$( _thing.insert($k, $v);)*//~ ERROR the type of this value must be known in this context
_thing
}}}pubfnmain(){use collections::treemap::TreeMap;let x:TreeMap<int,int> = map!(1 => 2);}
Overall I think it would be nicer to have the original syntax idea:
let s: HashSet = seq!(1, 2, 3); // HashSet impls Seq
let s: HashMap<int, int> = seq!((1, 10), (2, 20), (3, 30); // impls Seq<(int, int)>
But I have no clue what we would have to introduce to make that work.
That macro only works for Copy types, if you want to cover non-copy types then you need to replace the map(|&x| x) with map(|x| x.clone()), but then (for some reason) you need to type annotate the integers with a suffix (at least in the example above).
I don't claim that's the most efficient implementation, just want to show that it can be readily implemented today.
For it to be accepted into std we might need an efficient implementation though, and preferably one which works interchangeably with Copy and non-copy types (I guess?).
Be sure to implement a rules which allow for trailing commas. For example:
macro_rules! vec(($($e:expr),*) => ({// leading _ to allow empty construction without a warning.letmut _temp = ::std::vec::Vec::new();
$(_temp.push($e);)*
_temp
});($($e:expr),+,) => (vec!($($e),+)))
@gsingh93 There's a link to my out of tree implementation (i.e. a cargo crate) in the top comment of the RFC. However, I'm not maintaining the crate anymore - I've listed some alternatives in the README though. You can either use one of the listed alternatives or maintain a fork of my original crate.
Activity
TyOverby commentedon Jun 7, 2014
One problem that I can see off the top of my head is thatmacros.rsis in thestdcrate, while all the collections that you are using are in thecollectionscrate. I'm not sure if you could add macros to thecollectionscrate.I am strongly in favor of getting these macros added somewhere simply because of how common the collections are used. We need a shortcut.
schmee commentedon Jun 7, 2014
If this gets added, I'm strongly in favor of using
:instead of=>in the map macros since it'sTyOverby commentedon Jun 7, 2014
I'm not so sure about this. Colons are already used for type annotations and namespaces. I think
->or=>are perfectly acceptable.gsingh93 commentedon Jun 7, 2014
@TyOverby Didn't the collections get moved to std?
I'm fine with either syntax.
TyOverby commentedon Jun 7, 2014
Oh wow, they totally did. I wonder if Vec will get moved there too?
huonw commentedon Jun 7, 2014
Vechas always been accessible viastd?In any case, I think this is a change that requires an RFC. I would prefer having a generic macro (or two of them) that allows constructing any container type, rather than having to manually create one for each type and forcing other data structure implementers to do the same.
TyOverby commentedon Jun 8, 2014
I meant in
std::collections, but I suppose it's more useful incore.gsingh93 commentedon Jun 8, 2014
@huonw How would that generic macro figure out the type of container to create? Would you have to pass in the type?
I can create an RFC if no one else wants to (note: I don't want to, I wouldn't be able to comment on implementation at all).
huonw commentedon Jun 8, 2014
Using traits, something like
(I don't have time to write an RFC atm, maybe in a week and a half.)
gsingh93 commentedon Jun 8, 2014
Ah, I see. I'll post in this thread next week then to ask about the RFC unless anyone else gets around to writing it.
gereeter commentedon Jun 8, 2014
Isn't
seq!already implementable using theFromIteratortrait?huonw commentedon Jun 9, 2014
@gereeter yes, but it would be quite ineffficient.
tomjakubowski commentedon Jun 9, 2014
@huonw Would that macro work? I tried to write a similar one for the
MutableMaptrait which wouldn't type check because the type of_thingmust be known at theinsert:huonw commentedon Jun 9, 2014
Ah, looks like Rust's current type inference might not be powerful enough (I have no idea if we extend it to make it work).
Something like
($ty: type: $($k: expr ... ))expanding tolet mut _thing: $ty = ...might work; used likemap!(TreeMap<int, int>: 1 => 2).(With this, even
map!(TreeMap<_, _>: 1 => 2)might work.)1 remaining item
treeman commentedon Jul 27, 2014
I was thinking about something like this. I agree that something generic would be preferred.
We would need to figure out which syntax we would like.
This is a working implementation @huonw mentioned: playpen
Which would give a syntax like:
Another alternative would be to have a syntax like:
However I could not get the macro to recognize
mut. This is what I have currently:Overall I think it would be nicer to have the original syntax idea:
But I have no clue what we would have to introduce to make that work.
treeman commentedon Jul 27, 2014
It seems like RFC #143 may allow something similar.
japaric commentedon Jul 27, 2014
That macro can be implemented using what we have today without using any new trait:
That macro only works for
Copytypes, if you want to cover non-copy types then you need to replace themap(|&x| x)withmap(|x| x.clone()), but then (for some reason) you need to type annotate the integers with a suffix (at least in the example above).I don't claim that's the most efficient implementation, just want to show that it can be readily implemented today.
treeman commentedon Jul 28, 2014
@japaric that's nice, thanks!
For it to be accepted into
stdwe might need an efficient implementation though, and preferably one which works interchangeably withCopyand non-copy types (I guess?).thestinger commentedon Aug 4, 2014
It should also make use of
with_capacity. I suggest extending the container trait to includewith_capacityand a genericputfunction.brendanzab commentedon Aug 8, 2014
Be sure to implement a rules which allow for trailing commas. For example:
japaric commentedon Aug 8, 2014
I've crafted a
seq!macro (see example in the playpen!) that meets all the requirements stated in this thread:Seqtrait and works with the inference engine (as proposed by @huonw)[1, 2, 3]and a map{1 => 2, 2 => 3}style (as suggested by @treeman)with_capacityto reduce reallocations (as suggested by @thestinger)I'm leaving a (minimal) copy of the code here for posteriority:
What do you guys think of this implementation?
treeman commentedon Aug 17, 2014
Nice @japaric, I like it.
You could write an RFC for this if you want?
japaric commentedon Aug 21, 2014
@treeman I've submitted an RFC.
japaric commentedon Dec 24, 2014
@nick29581 Could you please move this issue to the RFC repo?
rust-highfive commentedon Dec 25, 2014
This issue has been moved to the RFCs repo: rust-lang/rfcs#542
gsingh93 commentedon Mar 12, 2015
@japaric It'd be nice if you could put the seq implementation in a crate so I could use it throughout my projects without copying and pasting.
japaric commentedon Mar 12, 2015
@gsingh93 There's a link to my out of tree implementation (i.e. a cargo crate) in the top comment of the RFC. However, I'm not maintaining the crate anymore - I've listed some alternatives in the README though. You can either use one of the listed alternatives or maintain a fork of my original crate.
Auto merge of rust-lang#14726 - Veykril:cistuff, r=lnicola