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

Audit use of Seq and try to make it less restrictive. #1131

Closed
jemc opened this issue Aug 17, 2016 · 13 comments
Closed

Audit use of Seq and try to make it less restrictive. #1131

jemc opened this issue Aug 17, 2016 · 13 comments

Comments

@jemc
Copy link
Member

jemc commented Aug 17, 2016

As discussed on the sync call, we want to audit the Seq interface, as it currently contains a lot of methods that make extra assumptions about what kind of sequence is being dealt with, whether it has a fixed size, whether the underlying pointer can be resized, etc.

In the upcoming value-dependent type changes, we will have a Vector type with a fixed size that won't be compatible with Seq as it currently stands.

I would suggest reducing the number of methods in Seq and possibly adding other interfaces to cover Seqs that have a size that isn't fixed, and the ability to reserve more space in the pointed-to-buffer.

@SeanTAllen
Copy link
Member

Is there anything this should be tagged as?

@jemc
Copy link
Member Author

jemc commented Aug 28, 2016

Thanks, I added some tags. This is ready for someone to pick up if they want to do an audit.

@Perelandric
Copy link
Contributor

Just an audit of usage in the standard library? Or anything else?

@jemc
Copy link
Member Author

jemc commented Aug 28, 2016

I'd say just do the standard library. If there are conflicts between the proposed new interfaces and users existing code, hopefully those will come to light in the RFC process.

@Perelandric
Copy link
Contributor

Perelandric commented Aug 28, 2016

@jemc: Sounds good, here's an overview. I can do something more in depth in a day or so to see where and how the various returned Seq instances are used.


Updated with uses of Seq methods from return values

Implemented explicitly by:

  • String
  • Array
  • List Includes the following "dummy" methods to satisfy Seq:
    • create
    • reserve

Used as return in interfaces (other than Seq):

  • promises.ReadlineNotify.tab()
    • Seq.size
    • Seq.apply
    • Seq.values

Used as return (or constraint on return) in concrete types:

  • encode.Base64.encode_url()
  • encode.Base64.encode()
    • Seq.create
    • Seq.push
    • Seq.append
    • Seq.clear
  • encode.Base64.decode()
  • encode.Base64.decode_url()
    • Seq.create
    • Seq.push
  • regex.Match.apply()
    • Seq.truncate
  • regex.Match.find()
    • Seq.truncate
  • regex.Match.replace()
    • Seq.truncate
    • Seq.reserve

Used as parameter (or constraint on parameter) in concrete types:

  • collections.Sort._sort() utilizes:
    • Seq.apply
  • collections.Sort._swap() utilizes:
    • Seq.apply
    • Seq.update

@SeanTAllen
Copy link
Member

@Perelandric thanks for the first pass audit. have you had a chance to dig in any further?

@Perelandric
Copy link
Contributor

@SeanTAllen My attempt so far is probably naive. I've merely searched for explicit uses of Seq in the standard library, and noted those above. At this point I think I'd need a little direction if more data is needed.

@jemc
Copy link
Member Author

jemc commented Sep 28, 2016

Next step would be for someone to confirm which methods of Seq match Vector in ponyta and draw up an RFC that reduces the surface of Seq to those methods, and creates an additional interface that extends Seq to cover the other (resize/reserve-related) methods that Seq currently has. Some methods could be removed from the interfaces entirely if they are not used or are not general enough.

@SeanTAllen
Copy link
Member

@Perelandric do you want to continue on with this or should I reach out to the dev list and see if anyone wants to take it on?

@Perelandric
Copy link
Contributor

@SeanTAllen Please put it out on the list. Thanks.

@SeanTAllen
Copy link
Member

@Perelandric email sent

@SeanTAllen
Copy link
Member

@jemc can you write up an issue on the RFC repo requesting the RFC and we can close this one out. I'm working on other RFCs now.

@jemc
Copy link
Member Author

jemc commented Oct 22, 2016

Closed in favor of ponylang/rfcs#61.

@jemc jemc closed this as completed Oct 22, 2016
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

No branches or pull requests

3 participants