Skip to content

Change/simplify projections #231

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

Closed

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Nov 17, 2014

More aggressive relimination of

Lambda { Apply = ... } # Apply

types. Review by @adriaanm @DarkDimius

* refinement type `T { X = U; ... }`
*/
def reduceProjection(implicit ctx: Context): Type = {
val reduced = prefix.lookupRefined(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe replace body with prefix.lookupRefined(name) orElse this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be valid and more elegant. But reduceProjection is called often, so to not have unforeseen performance degradations we tend to eschew object allocations in hot paths. Can and should come back to this once we have macros or guaranteed inlining. But then it's likely to be a global cleanup by means of a regexp search.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough. Maybe a // TODO: use Type's orElse combinator once it is inlined then?

@adriaanm
Copy link
Contributor

Can (should?) this also reduce T { type X = U; type U = V; ... } # X to V if V does not refer to the refinement's self variable?

@odersky
Copy link
Contributor Author

odersky commented Nov 17, 2014

The current implementation should do this. I'll think of a way to demonstrate this.

@adriaanm
Copy link
Contributor

Great!

An example where this helps:

Previously, the private value `mnemonics` in Coder.scala was fof the form

     Lambda$IP { ... } # Apply

It now simplifies to a Map[...] type.
Needed some fixes to lookup refined. The potential alias
type is now calculated by taking the member of the original
refined type, instead of by simply following the refined info.
This takes into account refinements that were defined after
the refinement type that contains the alias.

The change amde another test (transform) hit the deep subtype limit,
which is now disabled.
@odersky odersky force-pushed the change/simplify-projections branch from 71d3b36 to 8daa7a0 Compare November 18, 2014 17:08
@odersky
Copy link
Contributor Author

odersky commented Nov 18, 2014

Force pushed after rebasing

@smarter
Copy link
Member

smarter commented Nov 18, 2014

I think you accidentally force-pushed every branch in https://github.com/dotty-staging/dotty/

@odersky
Copy link
Contributor Author

odersky commented Nov 18, 2014

On Tue, Nov 18, 2014 at 7:41 PM, Guillaume Martres <notifications@github.com

wrote:

I think you accidentally force-pushed every branch in
https://github.com/dotty-staging/dotty/

Oh crap! How do we fix this?

  • Martin

Reply to this email directly or view it on GitHub
#231 (comment).

Martin Odersky
EPFL

@DarkDimius
Copy link
Contributor

@smarter if you'll tell me which branches you want reverted before force-push I can revert them.

@sjrd
Copy link
Member

sjrd commented Nov 18, 2014

Well, I would typically answer git reflog, but you can't access the reflogs on the server.
Each person can however recover the branch he last pushed correctly, since their local copy will still have the correct tip. If that was fetched locally, then local reflogs can be used to recover those things.
As a last resort, git fsck can sometimes recover things that were lost even to reflog, assuming no git gc was run since.

@adriaanm
Copy link
Contributor

You can also ask github to disable push --force. We did that in scala/scala and have never needed it.

@DarkDimius
Copy link
Contributor

@adriaanm we actually are rewriting history for dotty-staging. But disabling force-pushing for dotty master would indeed be a good step I guess.

@smarter
Copy link
Member

smarter commented Nov 18, 2014

@DarkDimius: There's no branch I want to revert particularly, I just wanted people to be aware of this, for historical purpose it might be worth reverting every branch (except the one this pull-request is based on obviously).
@odersky : If you want to always push only the current branch you're in, you can set git config --global push.default simple (default behavior of Git >=2.0).

@retronym
Copy link
Member

Seem like you can also recover the overwritten branch tips from the event log:

https://gist.github.com/retronym/7ccec6d7ad0f0bb0db50

Hat tip:

http://www.objectpartners.com/2014/02/11/recovering-a-commit-from-githubs-reflog/

Love the first comment on that blog:

Excellent. You can almost hear the people saying “This is exactly why Git is so [great, terrible]“

@sjrd
Copy link
Member

sjrd commented Nov 19, 2014

Oh nice one!

@VladUreche
Copy link
Contributor

Nice!

@namin
Copy link
Contributor

namin commented Nov 19, 2014

💯

@odersky
Copy link
Contributor Author

odersky commented Nov 26, 2014

Superseded by #232

@odersky odersky closed this Nov 26, 2014
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this pull request May 8, 2025
Backport "Handle type aliases in contextFunctionResultTypeAfter" to 3.3 LTS
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.

8 participants