Fix AST transformations of by-name parameters #27

smithjessk · 2016-07-06T19:46:47Z

Closes #24.

Stop canonicalization if parameters are by-name. This works even if the function has variable arguments
Add tests to cover these cases. These tests are inspired by / copied from Scala Async.
Correct the implementation of a test also from Scala Async

…into add-async-tests

… scope

…into add-async-tests

However, this commit is not stable. There is an infinite loop when compiling `async-await-tests.scala`.

smithjessk/add-async-tests contains many async tests that do not pass. Some of these, along with some improvements to the code, were copied into this branch. This commit removes the changes and tests that have to do with method definitions.

axel22 · 2016-07-09T09:30:08Z

project/Build.scala

@@ -96,6 +96,7 @@ object CoroutinesBuild extends MechaRepoBuild {
      "org.scalatest" % "scalatest_2.11" % "2.2.6" % "test",
      "org.scala-lang.modules" %% "scala-parser-combinators" % "1.0.2",
      "org.scala-lang" % "scala-reflect" % "2.11.4",
+      "org.scala-lang" % "scala-compiler" % "2.11.4",


How come we need this dependency?

I added it while experimenting with different solutions. Forgot to take it out.

Removed it in 1387ecb.

axel22 · 2016-07-09T11:17:24Z

Please also add an additional file to tests by-name-tests.scala, using the ScalaTest template from another test file, and add multiple unit tests for the functionality that you are implementing.

smithjessk · 2016-07-11T17:53:24Z

Yes! Will do.

To-do list for unit tests:

disallow coroutine applications as by-name parameters (2b576cf, 570221f)
repeated parameters handled correctly (ffd0ea7)
by-name parameters not canonicalized (28adad9, b2983ad)

…eters Note that these tests currently fail.

Assert that they are not lifted even when they surround a non by-name parameter.

axel22 · 2016-07-22T18:35:40Z

Should I take another look?

smithjessk · 2016-07-26T18:50:41Z

I still have to go over a few of your comments. In the meantime, could you look at this? Thanks!

Previously, the parameters passed by-name were unapplied coroutines.

smithjessk · 2016-07-26T20:04:44Z

To-do list for by-name analysis:

Use ByNameParam class as described here

axel22 · 2016-08-20T00:28:18Z

Note: this branch has merge conflicts.

smithjessk · 2016-08-24T01:27:45Z

Yep- I'm still working on it. Do you mind taking a look at this comment?

smithjessk added 17 commits June 2, 2016 17:18

Partially implement reflection test from Scala Async

aae630a

Merge branch 'master' of https://github.com/storm-enroute/coroutines …

647d1ea

…into add-async-tests

Copy method definitions into canonicalized AST

c354504

Copy declarations and identifiers from method definitions into proper…

20ce237

… scope

Merge branch 'master' of https://github.com/storm-enroute/coroutines …

4136755

…into add-async-tests

Remove checks that asserted equality between unit types

a9a7885

Make some progress on properly transforming by-name parameters

e07ca52

However, this commit is not stable. There is an infinite loop when compiling `async-await-tests.scala`.

Remove infinite looping introduced in the previous commit

36ef738

Merge master

23bf279

Re-add tests that were accidentally removed in previous commit

c2f47df

Don't canonicalize nested methods

db52258

Fix failing tests involving by-name parameters

9ade443

Remove failing and commented-out tests

1e78d94

Document and simplify canonicalization of function application

c35b6b7

Clean up and document by-name parameter transformation

c476e7d

Fix implementation of test from Scala Async

bd1c75a

axel22 reviewed Jul 9, 2016
View reviewed changes

smithjessk added 2 commits July 11, 2016 13:23

Remove dependency on scala-compiler

1387ecb

Clean up by-name modifications in AstCanonicalization

1bc8ded

smithjessk added 5 commits July 11, 2016 14:39

Remove commented-out code

06e28eb

Add tests to assert that coroutines cannot be passed as by-name param…

2b576cf

…eters Note that these tests currently fail.

Add repeated parameters test to ByNameTest

ffd0ea7

Add test of by-name parameter traversal

28adad9

Assert that they are not lifted even when they surround a non by-name parameter.

Add another by-name parameter test

b2983ad

smithjessk added 2 commits July 26, 2016 16:02

Pass coroutine applications by-name in ByNameTest

570221f

Previously, the parameters passed by-name were unapplied coroutines.

Run NestedContextValidator on by-name parameters

fa55257

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix AST transformations of by-name parameters #27

Fix AST transformations of by-name parameters #27

smithjessk commented Jul 6, 2016 •

edited

Loading

axel22 Jul 9, 2016

smithjessk Jul 11, 2016

axel22 commented Jul 9, 2016

smithjessk commented Jul 11, 2016 •

edited

Loading

axel22 commented Jul 22, 2016

smithjessk commented Jul 26, 2016

smithjessk commented Jul 26, 2016

axel22 commented Aug 20, 2016

smithjessk commented Aug 24, 2016

Fix AST transformations of by-name parameters #27

Are you sure you want to change the base?

Fix AST transformations of by-name parameters #27

Conversation

smithjessk commented Jul 6, 2016 • edited Loading

axel22 Jul 9, 2016

Choose a reason for hiding this comment

smithjessk Jul 11, 2016

Choose a reason for hiding this comment

axel22 commented Jul 9, 2016

smithjessk commented Jul 11, 2016 • edited Loading

axel22 commented Jul 22, 2016

smithjessk commented Jul 26, 2016

smithjessk commented Jul 26, 2016

axel22 commented Aug 20, 2016

smithjessk commented Aug 24, 2016

smithjessk commented Jul 6, 2016 •

edited

Loading

smithjessk commented Jul 11, 2016 •

edited

Loading