Skip to content

Allow MiniPhase to be DenotTransformer & LazyVals #81

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 7 commits into from
Mar 20, 2014

Conversation

DarkDimius
Copy link
Contributor

Finally :-)
@odersky, @sjrd please review

odersky added 5 commits March 18, 2014 16:06
This makes naming uniform between trees and types, and also avoids the clash with
transform.TreeTransformer. The idea is that transformers are parts of phases, and
have logic that is phase-specific. In particular, a context is passed around when
transforming a tree. Maps are simpler, they only have a T -> T apply method.
To bring in line with TreeTransformer terminology.
Many small and large changes. Added samplePhase to demonstrate functionality.

To test functioning, run the compiler with args

     tests/pos/uncurry.scala -Ylog:sample,terminal
CreateCompanionObjectTests were re-enabled. They failed in a previous version but succeed again in the latest commit.
Also uncurry.scala got renamed to curried.scala.
@@ -61,7 +61,7 @@ class tests extends CompilerTest {
@Test def dotc = compileDir(dotcDir + "tools/dotc", twice)
@Test def dotc_ast = compileDir(dotcDir + "tools/dotc/ast", twice)
@Test def dotc_config = compileDir(dotcDir + "tools/dotc/config", twice)
@Test def dotc_core = compileDir(dotcDir + "tools/dotc/core", twice)
@Test def dotc_core = compileDir(dotcDir + "tools/dotc/core", twice, xerrors = 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

error: _id is not a member of T?
            block._id = trans.head._id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing to public accessor doesn't help:
See here: https://travis-ci.org/lampepfl/dotty/builds/21027367#L2878 , line 2878

Creates accessors for lazy vals:
1) lazy local vals are rewritten to dotty.runtime.Lazy*** holders
2) for a non-volatile field lazy val create a non-thread-safe accessor and flag:
2.a) if lazy val type indicates that val is not nullable, uses null value as a flag
2.b) else uses boolean flag for sake of performance, method size, and
allowing more jvm optimizations
3) for a volatile field lazy val use double locking scheme, that guaranties no
spurious deadlocks, using long bits as bitmaps and creating companion
objects to store offsets needed for unsafe methods.

Conflicts:
	test/dotc/tests.scala
@DarkDimius
Copy link
Contributor Author

@sjrd @odersky I've tried to incorporate comments into commits. Please have a one more look.

@sjrd
Copy link
Member

sjrd commented Mar 19, 2014

I believe the commit message of the last commit is outdated.

@sjrd
Copy link
Member

sjrd commented Mar 19, 2014

OK that's all for me ;-) Only minor things except for the commit message.

All MiniPhases now as are full-fledged phases,
and are given their own periods and can register DenotTransformers.
MiniPhases belonging to same group(list) will be squashed to single phase.
@DarkDimius
Copy link
Contributor Author

I've updated the commit message.

@odersky
Copy link
Contributor

odersky commented Mar 20, 2014

LGTM now. Thanks for the fixups!

odersky added a commit that referenced this pull request Mar 20, 2014
 Allow MiniPhase to be DenotTransformer & LazyVals
@odersky odersky merged commit 2df29a2 into scala:master Mar 20, 2014
@DarkDimius DarkDimius mentioned this pull request Mar 20, 2014
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this pull request Mar 19, 2025
Backport "Make sure definition tree has the defined symbol" 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.

3 participants