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

Prelude: add functions version of built-in operators #1037

Merged
merged 12 commits into from Oct 6, 2020

Conversation

TristanCacqueray
Copy link
Collaborator

This change adds the add function for Natural.

This change adds the `add` function for Natural.
@sjakobi
Copy link
Collaborator

sjakobi commented Jul 13, 2020

Could you say something about the motivation for this addition, @TristanCacqueray?

@TristanCacqueray
Copy link
Collaborator Author

This can be use to do function composition with addition, for example to generate a list of natural starting from one, we could write:

Prelude.List.generate count Natural (Prelude.Natural.add 1)

Instead of:

Prelude.List.generate count Natural (\(idx : Natural) -> idx + 1))

@Gabriella439
Copy link
Contributor

@TristanCacqueray: I think the main issue is that if we add this we would need to add functions for other operators for consistency, but then we'd run into naming conflicts (since Prelude.Bool.or is already taken, for example)

@TristanCacqueray
Copy link
Collaborator Author

I see, then I guess it's better to keep operator out of the Prelude.

@lisael
Copy link
Contributor

lisael commented Jul 15, 2020

Maybe we could add a Prelude.Operator package to namespace the operator functions? Or Prelude.<BuiltinType>.Operator ?

@Gabriella439
Copy link
Contributor

@lisael: Yeah, I would be fine with something like that!

@TristanCacqueray
Copy link
Collaborator Author

Alright I can work on that, so:

  • Prelude.Operator.add (+)
  • Prelude.Operator.mul (*)
  • Prelude.Operator.or (||)
  • Prelude.Operator.and (&&)
  • Prelude.Operator.equal (==)
  • Prelude.Operator.notEqual (!=)
  • Prelude.Operator.append (#)

@sjakobi
Copy link
Collaborator

sjakobi commented Jul 15, 2020

+1/2 for Prelude.Operator :)

There are some existing names that we can but don't have to reuse in the grammar:

; Nonempty-whitespace to disambiguate `http://a/a?a`
equivalent-expression = import-alt-expression *(whsp equivalent whsp import-alt-expression)
import-alt-expression = or-expression *(whsp "?" whsp1 or-expression)
or-expression = plus-expression *(whsp "||" whsp plus-expression)
; Nonempty-whitespace to disambiguate `f +2`
plus-expression = text-append-expression *(whsp "+" whsp1 text-append-expression)
text-append-expression = list-append-expression *(whsp "++" whsp list-append-expression)
list-append-expression = and-expression *(whsp "#" whsp and-expression)
and-expression = combine-expression *(whsp "&&" whsp combine-expression)
combine-expression = prefer-expression *(whsp combine whsp prefer-expression)
prefer-expression = combine-types-expression *(whsp prefer whsp combine-types-expression)
combine-types-expression = times-expression *(whsp combine-types whsp times-expression)
times-expression = equal-expression *(whsp "*" whsp equal-expression)
equal-expression = not-equal-expression *(whsp "==" whsp not-equal-expression)
not-equal-expression = application-expression *(whsp "!=" whsp application-expression)

Also, don't forget ++.

@TristanCacqueray
Copy link
Collaborator Author

So Operator.TextAppend (++) and Operator.ListAppend (#) ?

@Gabriella439
Copy link
Contributor

Another option is that we could name the fields after the operators by quoting the operators with bacticks. In other words, you could do:

Prelude.Operator.`==`

However, I'd also be fine with the current names, too

@lisael
Copy link
Contributor

lisael commented Jul 17, 2020

Another shower thought is that the operators that are not commutative (namely ++ and #) should provide a reversed order function too, to permit currying in both orders.

let appendText
    : Text  Text  Text
    = λ(m : Text)  λ(n : Text)  n ++ m

let prependText
    : Text  Text  Text
    = λ(m : Text)  λ(n : Text)  m ++ n

Note that the name of the function becomes verbSubject and that appendText is n ++ m while textAppend was m ++ n.

Usage:

let hostnameToFqdn = λ(hostname : Text)  appendText ".example.com" hostname

@sjakobi
Copy link
Collaborator

sjakobi commented Jul 17, 2020

Another option is that we could name the fields after the operators by quoting the operators with bacticks. In other words, you could do:

Prelude.Operator.`==`

I love this idea! This eliminates the problem that some of the current names might be hard to remember. For example add could also be named plus.


@lisael I think we need to find a balance between adding quality-of-life improvements and keeping the Prelude at a size where it's not overly hard to navigate and maintain. Performance is also a relevant concern.

When considering API enhancements in Haskell libraries I sometimes try use the Fairbairn threshold.

So I'm not a fan of adding flipped versions of functions.

We could rather consider adding something like

Prelude.Function.flip : (a : Type)  (b : Type)  (a  b)  b  a

if anyone would want to use that.


Also, do we really want to have a version with the .dhall file extension and another one without for each export? I thought keeping the files without the extension was just a backward compatibility measure for the existing API.

@TristanCacqueray TristanCacqueray changed the title Prelude: Add Natural.add function Prelude: add functions version of built-in operators Jul 19, 2020
@TristanCacqueray
Copy link
Collaborator Author

Alright, next commit uses the operator symbols without the extensionless files.

@sjakobi
Copy link
Collaborator

sjakobi commented Jul 19, 2020

Thanks @TristanCacqueray! :)

I'm vaguely concerned about filenames like *.dhall causing problems – but maybe I shouldn't be!?

Are there any filename experts present? ;)

@sjakobi
Copy link
Collaborator

sjakobi commented Jul 19, 2020

https://stackoverflow.com/a/35352640/1013393 indicates that *.dhall [EDIT: and ||.dhall] might indeed cause problems on Windows. Could any Windows users confirm?!

@TristanCacqueray
Copy link
Collaborator Author

I named the file after the symbols as a test, but even if that works, I think that can be a source of unexpected problems. So either we pick type neutral name for the filename (and keep the operator symbols in the package.dhall record), either we could pack all the definition directly in the package.dhall record.

@Gabriella439
Copy link
Contributor

@TristanCacqueray: I think it would be fine to pack all of the definitions directly in the package.dhall record, with a comment explaining why

@TristanCacqueray
Copy link
Collaborator Author

I haven't check how the single file doc render though. Are the dhall-docs output available through hydra build logs?

@Gabriella439
Copy link
Contributor

@TristanCacqueray: Not for this repository. We build dhall-docs sample output in CI for the dhall-haskell repository

Copy link
Collaborator

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good! Nearly done, I think.

Prelude/Operator.dhall Outdated Show resolved Hide resolved
Prelude/Operator.dhall Outdated Show resolved Hide resolved
@sjakobi
Copy link
Collaborator

sjakobi commented Jul 22, 2020

Maybe it would be more conventional to offer these functions via Prelude/Operator/package.dhall instead of Prelude/Operator.dhall though.

@TristanCacqueray
Copy link
Collaborator Author

Thank you all for the feedbacks, I hope I get the suggestions right in the last commits :-) Please do let me know otherwise.

@Gabriella439 Gabriella439 merged commit 558280c into dhall-lang:master Oct 6, 2020
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

Successfully merging this pull request may close these issues.

None yet

5 participants