Deriving options #8258
Deriving options #8258
Conversation
| large >= large; | ||
|
|
||
| small >= large; | ||
| large <= small; |
There was a problem hiding this comment.
Should these have assertions of their truthiness or falsiness?
|
I'm a little worried that this is a lot of code to support a not-too-common case. Is there precedent for having these options in the I'm ok with implementing custom behavior myself, but I may not be a good sample of the opinion at large. Regardless, I'm not sure that |
| { // restrict the lifetime of these immutable borrows. | ||
| let mut check_valid = self.test_order.iter() | ||
| .chain_(self.ignore.iter()) | ||
| .chain_(self.reverse.iter()); |
|
As usual though, good clean code with good tests! |
|
I guess I'll voice some opposition to this direction. Deriving doesn't need to be such a general mechanism. If one has special needs for how Eq is implemented, then it isn't that painful to write it by hand. The linked issue provides little justification that this is warranted. This is another compiler-integrated feature that will need to be specced, but will it ever be used, and will the amount of use it gets justify the complexity? I'm not convinced. |
Previously it would call:
f(sf1.cmp(&of1), f(sf2.cmp(&of2), ...))
(where s/of1 = 'self/other field 1', and f was
std::cmp::lexical_ordering)
This meant that every .cmp subcall got evaluated when calling a derived
TotalOrd.cmp.
This corrects this to use
let test = sf1.cmp(&of1);
if test == Equal {
let test = sf2.cmp(&of2);
if test == Equal {
// ...
} else {
test
}
} else {
test
}
This gives a lexical ordering by short-circuiting on the first comparison
that is not Equal.
For example, in #[deriving(SomeTrait(foo, bar="baz", qux))], the bracketed data will get passed to the SomeTrait implementation. It supports both the list notation and the #[deriving(SomeTrait="a literal")] notation.
…al structs.
Example:
#[deriving(Eq(test_order(y), ignore(z, w)),
TotalOrd(reverse(x)))]
struct Foo {
x: uint,
y: uint,
z: uint,
w: uint
}
This allows several things to be controlled:
- `test_order` prioritises certain fields (e.g. the cheapest tests be placed
first), i.e. y is tested first, and then x. This will not change the result of
`Eq` & `TotalEq` impls, but can change `Ord` & `TotalOrd`
- `reverse` reverses the result of Ord & TotalOrd comparisons for the given fields
(so it is as if x = 1 > x = 2 in the example above)
- `ignore` completely removes the fields from the comparison, e.g. `z` and `w` are
not considered at all for the `Eq` impl of `Foo` above.
Only `Eq` and `Ord` accept `ignore` (since a `Total*` impl ignoring some fields
would not be total), and only `Ord` and `TotalOrd` accept `reverse`. All 4 traits
accept `test_order`.
Any other option, or using a syntax different to `#[deriving(Trait(opt(field1,
field2, ..)))]`, is an error. Using any of these options on an enum, or a tuple or
unit struct is also an error.
|
I've split the general bug fixes into #8285, so this is now purely the controversial commits ( I'm addressing the comments now; and there's definitely no problem for this to be r-'d. I'm happy to simplify (e.g. removing (FWIW, I'm pretty sure Haskell doesn't support anything like this for its |
|
I still like this. The more work the compiler can do for me, the less work I have to do, esp when adding fields etc. |
|
I agree with the concerns raised by @brson and @alexcrichton , so I'm going to close this. |
Some general clean-up relating to deriving: - `TotalOrd` was too eager, and evaluated the `.cmp` call for every field, even if it could short-circuit earlier. - the pointer types didn't have impls for `TotalOrd` or `TotalEq`. - the Makefiles didn't reach deep enough into libsyntax for dependencies. (Split out from rust-lang#8258.)
This adds the infrastructure to allow
#[deriving]to take options (closes #7229), e.g.It also implements three options for the comparison traits
test_order,ignoreandreverse, e.g.:which means that the field
yis tested first,zandwaren't tested at all (forEq), andxis sorted in reverse order forTotalOrd.For the comparison traits, the options are strictly checked; it is an error to:
test_order,Eq&Ordsupportignore, andOrd&TotalOrdsupportreverse.)Trait(option(bar, baz))(This also adds documentation to the manual about this.)
At the moment, all other traits emit a warning if any options are used (and ignore them).
(In general, all forms of options are supported, e.g.
#[deriving(Foo="bar")]works too.)