Skip to content

Fix #3248: Handle repeated arguments in results of unapply selectors #3747

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 12, 2018
Merged

Fix #3248: Handle repeated arguments in results of unapply selectors #3747

merged 2 commits into from
Jan 12, 2018

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jan 4, 2018

Normal unapply selectors can return results that have a repeated
argument as last element. This needs to be handled. This also subsumes
the previous way to express such arguments with unapplySeq.

In the long run, we should be able to do without unapplySeq altogether.
This is an intermediate step to get there.

@odersky
Fix scala#3248: Handle repeated arguments in results of unapply selec…
4025987 
…tors

Normal unapply selectors can return results that have a repeated
argument as last element. This needs to be handled. This also subsumes
the previous way to express such arguments with `unapplySeq`.

In the long run, we should be able to do without `unapplySeq` altogether.
This is an intermediate step to get there.
@odersky odersky mentioned this pull request Jan 4, 2018
Closed
@odersky
Copy link
Contributor Author

odersky commented Jan 4, 2018

This currently fails the exhaustivity checks. @liufengyun Can you see whether this is fixable?

@liufengyun
Copy link
Contributor

@odersky : The bug is fixed. There are 3 problems:

  1. The parent Seq is incorrectly instantiated for RepeatedParamClass, causing exception in typedSeqLiteral, due to baseType failure in elemType.
  2. SeqLiteral is not handled in pattern matcher.
  3. SeqLiteral is not handled in exhaustivity check.

@liufengyun liufengyun assigned odersky and unassigned liufengyun Jan 10, 2018
@odersky
Copy link
Contributor Author

odersky commented Jan 12, 2018

@liufengyun Looks good. Thanks for the fix!

@odersky odersky merged commit bd856dd into scala:master Jan 12, 2018
@liufengyun liufengyun deleted the fix-#3248-1 branch January 12, 2018 09:01
@nicolasstucki
Copy link
Contributor

nicolasstucki commented Jan 12, 2018 •
edited
Loading

This PR improved the performance of the compiler in http://dotty-bench.epfl.ch/.
Dotty has never been so fast :)

@odersky
Copy link
Contributor Author

odersky commented Jan 12, 2018

Wow! I looked at the changes and have no idea what could have caused this. Maybe Fengyun's changes to the pattern matcher?

@liufengyun
Copy link
Contributor

It's related to the machine restart, where Turbo Disable setting failed.

The machine powered off around Jan 10th for unknown causes. Today I restarted machine, ran the config to fixed the CPU frequency and disable Turbo. For some reason, the Turbo mode setting doesn't take effect.

I'll fix the setting and rerun the benchmarks for the last 2-3 merges.

@OlivierBlanvillain
Copy link
Contributor

OlivierBlanvillain commented Jan 18, 2018 •
edited
Loading

I'm tagging this PR with a "needs spec" as it affects what's written here.

@allanrenucci
Copy link
Contributor

@OlivierBlanvillain I would open a new issue instead. It will probably get lost othwerwise

@OlivierBlanvillain OlivierBlanvillain mentioned this pull request Jan 18, 2018
Closed
This was referenced Jan 18, 2018
Closed
Merged
odersky added a commit to dotty-staging/dotty that referenced this pull request Feb 21, 2018
@odersky
Fix scala#3747: Spec changes to extractors
fe10bec
odersky added a commit that referenced this pull request Mar 15, 2018
@odersky
Merge pull request #4024 from dotty-staging/fix-#3747
8fe2b1d 
Fix #3861: Spec changes to extractors implemented in #3747
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Feb 26, 2019
@liufengyun
Revert scala#3747: it is not specified
ff41764 
cannot be written explicitly.
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Feb 26, 2019
@liufengyun
Revert scala#3747: it is not specified
05f94f1 
cannot be written explicitly.
liufengyun added a commit to dotty-staging/dotty that referenced this pull request Feb 26, 2019
@liufengyun
Revert scala#3747: it is not specified
819b709 
cannot be written explicitly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OlivierBlanvillain OlivierBlanvillain Awaiting requested review from OlivierBlanvillain

Assignees

@odersky odersky

Labels
stat:needs spec
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@odersky @liufengyun @nicolasstucki @OlivierBlanvillain @allanrenucci