Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Work on inference, unification and subject-reduction #328
Work on inference, unification and subject-reduction #328
Changes from 69 commits
f2bd761
8f068a0
a8b9916
50069df
17a6a50
465215c
9865cc8
ce199bc
d82b019
db583e2
e91ee90
588eddc
d369293
2706e78
212e1cc
7790644
b2b7d03
28e84a0
05996f8
0b8037a
e7b7bc7
e285838
e0d283d
05710a7
c051cef
542660b
f798b2a
27ceb83
5a3040c
a94a34b
95057aa
d5dd1ee
105d47d
036b8b5
f440308
4aa951e
f86ad7b
f727612
fccfd7c
b0e1bb9
d1e0561
cd798e5
759131a
6245738
d1f39eb
91a1452
f76704b
935bff8
a5c3611
abe7462
a188591
ee234b9
9c9ce7c
f1a94f1
6b532da
445be54
f6a3405
057d6a0
2d7742b
8277830
17c2d41
264dd66
81557c2
e5dd8a6
8a784d1
2af80c3
5eb7eda
07a3f39
d367273
0cc857f
a0e1f3e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I would keep the brackets for readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using equality here had a significant positive impact on performances if I remember well. Conversion on the other end can be very costly, and if it fails it is gonna be tested again partially by unification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not clear. We should compare the performances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't compare anymore because
eq
is in the wrong module now (cyclic depencency).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember well, this had a significant performance impact. Did you run benchmarks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that brackets were useful for readability here.