Skip to content

Fix #3213: Make PolyTypes generative #3239

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 2 commits into from
Oct 4, 2017
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Oct 3, 2017

Because PolyTypes are used as carriers of constrained parameter references,
we must make sure that we can duplicate them so that the duplicate is
different from the original. Hence, it's useless to cache PolyTypes.

Because PolyTypes are used as carriers of constrained parameter references,
we must make sure that we can duplicate them so that the duplicate is
different from the original. Hence, it's useless to cache PolyTypes.
@smarter
Copy link
Member

smarter commented Oct 3, 2017

I don't think you need to always make them generative, just when calling newLikeThis, I had discovered this issue in: #2981

@odersky
Copy link
Contributor Author

odersky commented Oct 4, 2017

I don't think you need to always make them generative, just when calling newLikeThis, I had discovered this issue in: #2981

The problem is that the constraints map will then re-merge polytypes because they are equal. I think making them all generative is the cleanest solution. Most of them are generative anyway since they contain a reference to a bound type parameter.

@odersky
Copy link
Contributor Author

odersky commented Oct 4, 2017

test performance please

@dottybot
Copy link
Member

dottybot commented Oct 4, 2017

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

dottybot commented Oct 4, 2017

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3239/ to see the changes.

Benchmarks is based on merging with master (432ceac)

@smarter smarter merged commit 72bf1ca into scala:master Oct 4, 2017
odersky added a commit to dotty-staging/dotty that referenced this pull request Oct 8, 2017
This is a partial revert from scala#3239. scala#3239 made both HKLambdas and
PolyTypes uncached. It turns out that this breaks things if we do it
for HKLambdas. I have not yet figured out why, however. The test case
in scala#3250 is a bit too large to be able to tell.
@allanrenucci allanrenucci deleted the fix-#3213 branch December 14, 2017 19:18
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