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

Add infix pattern for consing #609

Open
Bodigrim opened this issue Aug 10, 2024 · 9 comments
Open

Add infix pattern for consing #609

Bodigrim opened this issue Aug 10, 2024 · 9 comments

Comments

@Bodigrim
Copy link
Contributor

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.

@Lysxia
Copy link
Contributor

Lysxia commented Aug 10, 2024

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

@Bodigrim
Copy link
Contributor Author

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

@Bodigrim
Copy link
Contributor Author

Bodigrim commented Aug 10, 2024

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

@Bodigrim
Copy link
Contributor Author

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

@meooow25
Copy link
Contributor

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.

@Bodigrim
Copy link
Contributor Author

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.

@phadej
Copy link
Contributor

phadej commented Aug 13, 2024

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.

@meooow25
Copy link
Contributor

++ 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.

@Bodigrim
Copy link
Contributor Author

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.

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

4 participants