Remove lenses module from derived lenses

[Ralith]

Allows lenses to be derived for multiple structs in a single module without generating illegal duplicate module definitions.

I'd have preferred to make the lenses associated types, but there's no current way to accomplish that without also introducing ugly generated type names into the namespace that would inevitably pop up in error messages.

[Ralith]

#341 is an even better solution to this problem.

[cmyr]

Does it make sense for this to get merged anyway, while we discuss #341?

One other option for this, possible, would be to generate impl blocks for the type, and generate functions that return the lenses? So you would have CalcState::lens_value() or something. I'm not sure that's an improvement though, I'm just riffing.

[Ralith]

Ooh, that would work around the hygiene issues, at the cost of making the types anonymous.

[cmyr]

Yea, I'm not sure that's a huge concern?

To do associated types we'd need to create a trait for each lens?

[Ralith]

There's no way to do associated types without polluting the namespace with extraneous type and/or module definitions, which isn't necessarily a disaster but makes for bad error messages.

[cmyr]

I think we should get this in for now to unblock, and then we can go from there.

[cmyr]

looks like this needs a rebase though 😢

[Ralith]

Added a different approach, taking advantage of the loophole in rustc's public-in-private check that lets through things that are public only in a private module. This lets us use associated constants which, while still lacking externally accessible type names, are very convenient to reference.

[Ralith]

Hmm, error messages still aren't great:

error[E0277]: the trait bound `impl druid::Widget<f64>: druid::Widget<bool>` is not satisfied
  --> druid/examples/slider.rs:53:37
   |
53 |     col.add_child(Padding::new(5.0, slider), 1.0);
   |                                     ^^^^^^ the trait `druid::Widget<bool>` is not implemented for `impl druid::Widget<f64>`
   |
   = note: required because of the requirements on the impl of `druid::Widget<DemoState>` for `druid::lens::LensWrap<bool, __DemoState_druid_lenses::double, impl druid::Widget<f64>>`
   = note: required by `druid::widget::padding::Padding::<T>::new`

What do you folks think?

[cmyr]

If this is only ever accessed via the associated const, we're less concerned about having the actual name of the struct match the name of the field. I'm thinking about the possible hygene concerns about having a struct named, sat, value or inner or similar just sort of hanging out in the top level of a module? one option could be to create a per-type module to stash these in, so that you have something like,

//
#[derive(Lens)]
struct MyData {
    value: usize,
    name: String,
}

pub mod my_data_lenses {
    pub struct name;
    pub struct value;
}

impl MyData {
    const name: super::my_data_lenses::name = super::my_data_lenses::name;
}

again not sure this is necessary, just thinking out loud.

[Ralith]

That's actually exactly what this does! Rolling back to the original commit for now, though, since it's an unambiguous win. Will open a new PR for further discussion.

[cmyr]

okay this comment may be misguided, ignore as needed :)

[cmyr]

I think lenses are going to necessarily have some issues around how errors are presented. A lot of this will come down to how we explain lenses in the docs.

I think it's fair to expect that anybody who actually uses lenses will have had to read something; and as long as our error message can convey that the issue is with a generated lens, so that the user knows what questions to ask, I think that's acceptable.