text icon indicating copy to clipboard operation
text copied to clipboard

Add infix pattern for consing

Open Bodigrim opened this issue 1 year ago • 9 comments

We can add

pattern (:+) :: Char -> Text -> Text
pattern x :+ xs <- (uncons -> Just (x, xs)) where
  x :+ xs = cons x xs

which mimics what (:) does for lists. Presumably it could provide an easier migration route from String to Text.

Bodigrim avatar Aug 10 '24 17:08 Bodigrim

Can we name it :< instead and also add an Empty pattern? That would follow the precedent of Data.Sequence, where these appear in the following data type to be used with a ViewPatterns, presumably because it predates pattern synonyms:

data ViewL a = EmptyL | a :< Seq a

viewl :: Seq a -> ViewL a

Lysxia avatar Aug 10 '24 20:08 Lysxia

Yeah, aligning with Data.Sequence is a great idea. We'd probably also need infixr 5 as well.

Bodigrim avatar Aug 10 '24 21:08 Bodigrim

Not 100% sure that we need Empty pattern though. One can pattern match on "" (when OverloadedString are enabled) or [] (if OverloadedLists are).

Bodigrim avatar Aug 10 '24 21:08 Bodigrim

OTOH Empty would allow to declare {-# COMPLETE #-}, which would be nice.

Bodigrim avatar Aug 10 '24 21:08 Bodigrim

IMO a pattern in the cons direction might misleadingly indicate that it is a cheap operation like on lists or Seq.

It is a known pitfall for beginners elsewhere, for instance in Java, to write

String acc = "";
for (...) {
  acc += foo();
}

because += is cheap on numbers but not on strings.

meooow25 avatar Aug 12 '24 15:08 meooow25

I don't see why (:<) would imply it more than cons itself does. If users do not read documentation, there is not much we can do about it.

Bodigrim avatar Aug 12 '24 21:08 Bodigrim

My opinion that if you need consing to Text you are most likely doing something wrong. For String it's O(1), for Text it's O(n), and you probably should use Builder. Having an easy way to do wrong thing is dangerous. (my pet peeve: Using <> for Text is often silly, lazily written bad code. i.e. something like "foo (" <> textValue <> ") bar", which is not uncommon unfortunately)

EDIT: if someone migrates :-using String-code, they really ought to think what they are doing, and not just s/String/Text/g and expect cpu or/and memory usage improvements.

EDIT: not that ++ for String is any better than <> for Text, but at least it's quite easy to use ShowS with base, and get decent behavior without even needing to import any new modules. With text (and Builder) it's not at all as convenient.

phadej avatar Aug 13 '24 16:08 phadej

++ for String is in fact better than <> for Text: ++ is only linear in the left value so you suffer if you associate ++s the wrong way, but Text's <> is linear in both which means you are always inefficient if you chain <>s.

Also, this is perhaps a little off-topic, but code like "foo (" <> textValue <> ") bar" seem like good use cases for fusion (or some system of rewrite rules) to step in and improve the efficiency.

meooow25 avatar Aug 15 '24 15:08 meooow25

I understand performance considerations, but I'm not convinced. When migrating from String to Text I find it extremely useful to get quickly (and as mechanically as possible) something which compiles and passes tests, submit it, and then work on performance piece by piece. Otherwise you can waste lots of time carefully using Builders and redesigning algorithms only to hit a stumbling block later on.

Besides there are myriads of use cases where inefficience of "foo (" <> textValue <> ") bar" does not matter in a slightest way.

Bodigrim avatar Aug 21 '24 23:08 Bodigrim