Make cats.effect.std.PQueue share cats.effect.std.Queue trait

[skedlukecollier]

Currently PQueue extends PQueueSource and PQueueSink ran into a problem where trying to use fs2.Stream#fromQueueUnterminating led to a type error on a PQueue so following a convo on discord the trait's be linked or unified

[armanbilge]

Sorry, now that I think about it: I can see an argument that PQueue should not extend/implement the Queue trait, depending on what the Queue trait is promising. For example, if it is promising FIFO, then passing a PQueue could observably violate that expectation.

However, QueueSink and QueueSource specifically don't offer sufficient API surface to assert these sorts of laws. So in that case, it wouldn't seem unreasonable for PQueue{Source,SInk} to extend Queue{Source,Sink}. Their APIs are identical, both in terms of methods and semantics.

Actually, this makes me wonder if a better hierarchy might have been QueueSource, QueueSink, and Queue, with marker interfaces FifoQueue and PQueue. Then calling code could declare precisely what it does or doesn't need to work correctly.

Thoughts?

[skedlukecollier]

For the specific problem I had I think extending PQueueSource and PQueueSink with the QueueSink and QueueSource seems like the solution we're after!

For the next part would the Queue remain as is, then PQueue and a new FifoQueue extend Queue?

If I'm following then it would give types hints for these types of questions that Java faces due to its shared queue interface.

For me, the difference between a PQueue and Queue seems like a happy level of detail for most use cases here, 9/10 a Queue is probably fifo.

[armanbilge]

For the specific problem I had I think extending PQueueSource and PQueueSink with the QueueSink and QueueSource seems like the solution we're after!

Yes, I think so too! I think we can definitely make this change.

For the next part would the Queue remain as is, then PQueue and a new FifoQueue extend Queue?

Well, ideally we'd do this, but due to compatibility constraints we probably no longer can. i.e. the expectation is that Queue is a FIFO queue, so actually we need to add something "above" it in the hierarchy which may not be FIFO.

[skedlukecollier]

Yes, I think so too! I think we can definitely make this change.

Created #3733 for this, will update it to not be failing when I get chance

Well, ideally we'd do this, but due to compatibility constraints we probably no longer can. i.e. the expectation is that Queue is a FIFO queue, so actually we need to add something "above" it in the hierarchy which may not be FIFO.

I can't think of a word for an AbstractQueue perhaps that's a sign it's an unnecessary abstraction? 🤔

[armanbilge]

it's an unnecessary abstraction

true, at the moment we don't have a usecase for it.

[neomaclin]

added a PR #3930 to follow up on this

[durban]

Fixed by #3930.