Improve support for custom list object implementations #2985
Conversation
This file contains 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
ChrisJefferson
force-pushed
the
homogenous-list
branch
from
6cd1646 to
f6e7a8e
Compare
fingolfin
reviewed
Nov 9, 2018
ChrisJefferson
force-pushed
the
homogenous-list
branch
3 times, most recently
from
63f1720 to
590760d
Compare
|
Fixes made. I'm sure a more complete overhall of lots of list based stuff could be made (and reading comments in the source, many things were never finished in the first place), but this adds enough functionality for many basic cases, including the cases I want to work. |
fingolfin
reviewed
Nov 10, 2018
fingolfin
reviewed
Nov 10, 2018
Codecov Report
@@ Coverage Diff @@
## master #2985 +/- ##
==========================================
+ Coverage 85.21% 85.24% +0.02%
==========================================
Files 110 110
Lines 56857 56877 +20
==========================================
+ Hits 48450 48482 +32
+ Misses 8407 8395 -12
|
Make the _FILT_LIST functions be a no-op when the FiltList they use is 0, so they can be safely called on non-list types. 0 is the default value used for the FiltLists. It is also a valid tnum value, T_INT, but it would never make sense for one of the *_FILT_LIST functions to turn something into a T_INT.
ChrisJefferson
force-pushed
the
homogenous-list
branch
from
7fc95fe to
50dcc7d
Compare
This has long been the case for plists, this adds support for lists which are not plists to also be non-homogenous and strictly sorted.
This was already allowed in Sort. While Sort and ParallelSort both fail with an error if given a non-dense lists, there can exists lists which are dense but do not have the filter IsDenseList
ChrisJefferson
force-pushed
the
homogenous-list
branch
from
50dcc7d to
d4f61c5
Compare
fingolfin
approved these changes
Nov 11, 2018
PaulaHaehndel
added
the
release notes: to be added
fingolfin
changed the title
fingolfin
changed the title
fingolfin
changed the title
fingolfin
added
kind: enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a selection of fixes which I came up with while trying to implement an object which acts as a mutable list.
SortParallel requires IsDenseList. Sort and SortBy only require IsList and work fine, so unify with them.
IsSSortedList requires the list is homogenous. In the distant past this was required for all lists, and then the requirement was removed for plists, without removing it for plists.
Make sure we don't call _FILT_LIST on non-plists. To be honest I'm not totally happy with this and considered rewriting this bit of code to make it less fragile, but decided I didn't want to go down that rabbithole. This certainly makes things better than they were.