Closed
Conversation
Updated type definitions to use NonNull for guaranteed non-empty collections: - RootedDeterministicPath.rootBranches now uses NonNull (OMap ...) - DPBranch.DPSequence branch sets now use NonNull (OSet ...) Removed error cases that checked for empty sets: - Removed "invariant broken: rootBranches must be nonempty" error - Removed "invariant broken: both branch sets must be nonempty" error Updated all code using these types to work with NonNull: - smartBuildSequence now takes NonNull parameters - smartBuildRootedDeterministicPath now takes NonNull parameters - branchify returns NonNull sets - All construction sites wrap with impureNonNull - Parse functions updated to construct NonNull values - MaterializePath updated to handle NonNull conversions
Added helper functions to MyPrelude.Collections: - traverseNonNull: safely traverse NonNull containers - mapOSetNN: map over NonNull OSets Replaced impureNonNull uses with safer alternatives: - Use singletonNNSet instead of impureNonNull . singletonSet - Use mapOSetNN instead of impureNonNull . mapOSet - Use traverseNonNull for traverse operations on NonNull - Use singletonNNMap for singleton maps This reduces the use of impureNonNull and makes the code safer by using functions specifically designed for NonNull types.
Use pattern matching and ncons to build the NonNull result directly, avoiding the need for impureNonNull. The error case remains but is marked as impossible since NonNull values should never be empty.
The original implementation is simpler and more elegant. The use of impureNonNull is justified here because traversing a non-empty structure always produces a non-empty result.
- Fixed traverseNonNull to require MonoFoldable (f b) instead of (g b) - Removed Lift and NFData deriving instances from types using NonNull (RootedDeterministicPath, DPBranch, DeterministicPath, PointlikeDeterministicPath, NormalizedPath) - NonNull types don't have Lift or NFData instances, so can't be derived automatically Remaining work: Fix type mismatches where NonNull values need toNullable conversions
- Added toNullable() calls when passing NonNull values to functions expecting regular containers - Fixed mapToList usage on NonNull OMaps by adding toNullable before mapToList - Updated unionWith to work with NonNull OMaps using toNullable/impureNonNull - Converted traverse operations on NonNull OSets to use toList/setFromList pattern - Updated parsers to return NonNull types (pSequenceBranchSet, pRootedBranchSet) - Fixed test specs to use singletonNNMap and singletonNNSet
- Updated one test case to use impureNonNull and singletonNNSet Note: Many test cases in ParseSpec still need updates to use impureNonNull . setFromList for DPSequence arguments. This is because DPSequence now takes NonNull (OSet (DPBranch a)) instead of OSet (DPBranch a). Additionally, TH file needs Lift instances which were removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updated type definitions to use NonNull for guaranteed non-empty collections:
Removed error cases that checked for empty sets:
Updated all code using these types to work with NonNull: