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

fix 234, fix 236, bugs related to parametric types #237

Merged
merged 1 commit into from
Oct 5, 2015
Merged

fix 234, fix 236, bugs related to parametric types #237

merged 1 commit into from
Oct 5, 2015

Conversation

EliasC
Copy link
Contributor

@EliasC EliasC commented Oct 3, 2015

The problem causing 234 was that methods from included traits did not
have their type parameters instantiated when being looked up.

The problem causing 236 was a weird call to replaceTypeVars when
typechecking new.

@albertnetymk
Copy link
Contributor

One minor comment: matchMethod was introduced to abstract the logic of finding matched method. Other that that, this PR looks OK to me.

@EliasC
Copy link
Contributor Author

EliasC commented Oct 4, 2015

I thought I'd reduce the ambiguity of the already overused concept of "matching". (==m) . mname is both readable and understandable in my opinion.

@albertnetymk
Copy link
Contributor

Explicit == doesn't seem the same level of abstraction with the rest body of the function. Unifying the method matching logic is handy once method overloading is added. Both points are rather subjective.

The problem causing 234 was that methods from included traits did not
have their type parameters instantiated when being looked up.

The problem causing 236 was a weird call to `replaceTypeVars` when
typechecking `new`.
@EliasC
Copy link
Contributor Author

EliasC commented Oct 4, 2015

Subjectiveness is ruining all the fun... Reverted now.

@albertnetymk
Copy link
Contributor

Sorry. My point is that it's subjective, so the author of PRs should hold the right to make the judgement call.

I am OK with the PR now. Merging?

@EliasC
Copy link
Contributor Author

EliasC commented Oct 4, 2015

I actually agree with you now (otherwise I wouldn't have changed it back). The overloading argument makes sense!

@albertnetymk
Copy link
Contributor

Merging it in 5 mins if no one objects.

@kikofernandez
Copy link
Contributor

looks good to me

kikofernandez pushed a commit that referenced this pull request Oct 5, 2015
fix 234, fix 236, bugs related to parametric types
@kikofernandez kikofernandez merged commit 573df3e into parapluu:master Oct 5, 2015
@kikofernandez kikofernandez deleted the fix/parametric-types-bugs branch October 5, 2015 14:55
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.

3 participants