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

SI-6457 ImmutableSetFactory.empty results in StackOverflowError #3366

Merged
merged 1 commit into from
Jan 15, 2014

Conversation

Ichoran
Copy link
Contributor

@Ichoran Ichoran commented Jan 15, 2014

ImmutableSetFactory relies upon empty to create its builder, which in turn relies upon its builder to create an empty. Added an emptyInstance method to hold the actual empty set (which also removes duplicated functionality for empty sets).

No test, as the original pattern:

object Factory extends collection.generic.ImmutableSetFactory[collection.immutable.Set] {}

no longer compiles. This should be verified by whoever checks this commit, but this kind of change is hard to revert by accident. No reason to waste resources checking it forevermore.

ImmutableSetFactory relies upon empty to create its builder, which in turn relies upon its builder to create an empty.  Added an emptyInstance method to hold the actual empty set (which also removes duplicated functionality for empty sets).

No test, as the original pattern:

  object Factory extends collection.generic.ImmutableSetFactory[collection.immutable.Set] {}

no longer compiles.  This should be verified by whoever checks this commit, but this kind of change is hard to revert by accident.  No reason to waste resources checking it forevermore.
@soc
Copy link
Contributor

soc commented Jan 15, 2014

This LGTM.

@retronym
Copy link
Member

LGTM, too

adriaanm added a commit that referenced this pull request Jan 15, 2014
SI-6457 ImmutableSetFactory.empty results in StackOverflowError
@adriaanm adriaanm merged commit 97be1dd into scala:master Jan 15, 2014
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