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

Simplify + when its arguments are equivalent #688

Closed
wants to merge 1 commit into from

Conversation

@sjakobi
Copy link
Collaborator

sjakobi commented Aug 1, 2019

No description provided.

@FintanH

This comment has been minimized.

Copy link
Collaborator

FintanH commented Aug 1, 2019

Why would this be useful, out of interest? :)

@sjakobi

This comment has been minimized.

Copy link
Collaborator Author

sjakobi commented Aug 1, 2019

I'm not fully convinced that this is even a worthwhile simplification, but I wanted to bring it up.

  1. It can shorten an expression involving complex arguments.
  2. It should speed up further evaluation of the normalized expression as the argument appears only once.
@FintanH

This comment has been minimized.

Copy link
Collaborator

FintanH commented Aug 1, 2019

It can shorten an expression involving complex arguments.

This is only the case if the two expressions end up beside each other though, right? Like it won't simplify x + y + x?

@sjakobi

This comment has been minimized.

Copy link
Collaborator Author

sjakobi commented Aug 1, 2019

This is only the case if the two expressions end up beside each other though, right? Like it won't simplify x + y + x?

Right. That would require exploiting commutativity. Doing that might be a good idea at some point – right now we don't even exploit associativity.

@FintanH

This comment has been minimized.

Copy link
Collaborator

FintanH commented Aug 1, 2019

Ya, so you're right. It'd be nice to get those kind of simplifications but it seems like it's not going to move that way, just yet.

I think it's a cool idea, but not sure if it is all that helpful as rule to be added to the standard grammar. But also looking forward to see what others think :)

@f-f

This comment has been minimized.

Copy link
Member

f-f commented Aug 1, 2019

I think so far we tended to keep the amount of rules as small as possible (even possibly sacrificing some performance) to keep the bar lower for new language implementations to happen as easily as possible, since the goal is to eventually integrate with every language.

This might change if for example we encode the rules in such a way that one can do code-generation from them (IIRC someone proposed encoding them in Dhall itself): then I think this would lower the cost of adding a single rule

@singpolyma

This comment has been minimized.

Copy link
Collaborator

singpolyma commented Aug 1, 2019

If we were going to do re-association and/or commutation this might be cool, but as is it seems unlikely to add value IMHO.

@Gabriel439

This comment has been minimized.

Copy link
Contributor

Gabriel439 commented Aug 2, 2019

Yeah, I think this would be more appropriate if it were part of a more systematic attempt to normalize arithmetic, but it's still not clear that we're ready for that, especially given that we've recently held off on implementing re-associating operators

@sjakobi

This comment has been minimized.

Copy link
Collaborator Author

sjakobi commented Aug 2, 2019

Thanks for the review! :)

@sjakobi sjakobi closed this Aug 2, 2019
@Gabriel439

This comment has been minimized.

Copy link
Contributor

Gabriel439 commented Aug 3, 2019

You're welcome! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.