Skip to content
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

over' and iover' pretend to work with incompatible optics #473

Open
treeowl opened this issue Dec 27, 2022 · 0 comments · May be fixed by #475
Open

over' and iover' pretend to work with incompatible optics #473

treeowl opened this issue Dec 27, 2022 · 0 comments · May be fixed by #475

Comments

@treeowl
Copy link
Contributor

treeowl commented Dec 27, 2022

ghci> length $ over' mapped id [undefined, undefined]
2

Oops. I'm currently working on adding over', iover', and relatives to lens. The choice I've made over there is not to add a Settable Solo instance, to avoid this very problem. I don't know how these optics work, but the uses of unwrapIdentity' in the Mapping (Star Identity') and Mapping (IxStar Identity') instances look really fishy. What goes wrong if those instances are removed, and over' and iover' are declared to need traversals rather than setters?

Additional surprising uses:

failover' and ifailover' both do things like

if visited
  then Just (unwrapIdentity' t)
  else Nothing

Should those be like this?

if | visited
   , Identity' () v <- t
   -> Just v
   | otherwise
   -> Nothing

or (using Solo)

if | visited
   , Solo v <- t
   = Just v
   | otherwise
   = Nothing

While you're in the ballpark, Identity' is quite an awkward type, and there's really no need for such complication.

Identity'  ~=  Solo -- Best Solo source is OneTuple, which handles all the compat mess
unwrapIdentity' ~= getSolo
wrapIdentity' ~= (Solo $!)
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
@treeowl treeowl linked a pull request Dec 28, 2022 that will close this issue
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
treeowl added a commit to treeowl/optics that referenced this issue Dec 28, 2022
* `over'`, `iover'`, `set'`, and associated operators previously
  accepted setters. However, it's impossible to actually modify strictly
  through a setter; a traversal is needed for that. Restrict the types
  to require `A_Traversal`, and remove the associated (technically
  correct but deceptive) `Mapping` instances.

* Document the strictness behavior of `set'`.

Fixes well-typed#473
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant