Skip to content

Fix Patmat and FunctionalInterfaces, enable 61 tests that succeed. #693

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

Merged
merged 11 commits into from
Aug 4, 2015

Conversation

DarkDimius
Copy link
Contributor

Several fixes to pattern matcher and FunctionalInterfaces, as well as fixes to backend. @odersky please review.

@odersky
Copy link
Contributor

odersky commented Jun 30, 2015

According to the gitter discussion, it would make sense to do a PR against 2.12 that drops the name sorting. When is the best time to do this?

@DarkDimius DarkDimius force-pushed the more-tests3 branch 2 times, most recently from 89642ec to 5dcec98 Compare July 1, 2015 11:09
DarkDimius referenced this pull request in AlexSikia/dotty Jul 1, 2015
Name wrangling was incorrect previously. Now uses the `specializedFor`
method in `NameOps`.
An incorrect return from a pattern match in `TypeSpecializer` resulted
in errors; test `genericClass_specialization` keeps an eye on those.
@odersky
Copy link
Contributor

odersky commented Jul 1, 2015

/rebuild

@DarkDimius
Copy link
Contributor Author

FATAL: Failed to fetch from https://github.com/lampepfl/dotty.git

/rebuild

@DarkDimius
Copy link
Contributor Author

/rebuild

1 similar comment
@DarkDimius
Copy link
Contributor Author

/rebuild

@DarkDimius
Copy link
Contributor Author

Passes all the tests on my local machine. Should be ready for review.
@odersky please have a look.

@adriaanm
Copy link
Contributor

/rebuild -- worker ran out of disk space

@DarkDimius
Copy link
Contributor Author

/rebuild

Includes fix to emission of invokeDynamic instructions
in positions where expected type isn't the type of lambda being returned.
SI-9387
@adriaanm
Copy link
Contributor

/rebuild (we set the timeout to 0, which jenkins interpreted as 3... ugh!)

def compare(x: Name, y: Name): Int = {
if (x.isTermName && y.isTypeName) 1
else if (x.isTypeName && y.isTermName) -1
else if (x.start == y.start && x.length == y.length) 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can just use

if (x eq y)

here because names are hash-consed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if (x eq y)

will not be true for TypeName('A') and TermName('A')

Copy link
Contributor

Choose a reason for hiding this comment

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

But you have already tested for that case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, agreed than. It wasn't the case initially.

@odersky
Copy link
Contributor

odersky commented Jul 16, 2015

Otherwise LGTM

@VladUreche
Copy link
Contributor

SpecializeNames: Duplicate scalac behaviour, sort tparams

@DarkDimius, why do you want to sort tparam names? Miniboxing got away without doing it and it never caused any problems.

@DarkDimius
Copy link
Contributor Author

@VladUreche this is code that uses Scala2 specialized functions, scalac sorts type params by their name to do name mangling:
https://github.com/scala/scala/blob/2.11.x/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala#L333

Use x.length - y.length trick,
Names are hash-consed.
@DarkDimius
Copy link
Contributor Author

@odersky comments addressed.

@odersky odersky merged commit 9bc9578 into scala:master Aug 4, 2015
@allanrenucci allanrenucci deleted the more-tests3 branch December 14, 2017 19:24
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.

4 participants