Skip to content

Conversation

@densh
Copy link
Contributor

@densh densh commented Dec 2, 2013

Resubmission of #3118 with additional bugfixes for SI-8008, SI-8016, SI-8009, SI-6842, SI-7979, SI-7980, SI-7996, SI-7789 and SI-7154. Best viewed per-commit.

review by @retronym @xeno-by

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 29725473)
🐱 Roger! Rebuilding pr-scala for 7d6b2be. 🚨

densh added 9 commits December 3, 2013 19:22
Previously it also matched other nodes but returned Nil as value of xs.
This behavior was added for sake of consistentcy with q”f[..$ts]”. On
the other hand q”f[..$Nil]” == q”f” but q”f(..$Nil)” == q”f()” not q”f”.
Due to this deconstruction/construction symmetry was broken.
Previously we believed that having Liftable outside of the Universe will
bring some advantages but it turned out this wasn’t worth it. Due to
infectious nature of path dependent types inside of the universe one
had to cast a lot. A nice example of what I’m talking about is a change
in trait ArbitraryTreesAndNames.

Additionally a number of standard Liftables is added for types that are
available through Predef and/or default scala._ import: Array, Vector,
List, Map, Set, Option, Either, TupleN.
This commit introduces internal attachment that allows unapply macros to
be aware of their sub patterns and tweak their expansion depending
on that info.

At the moment this is not possible due to the way pattern macros are
expanded:

	case MacroPat(inner1, inner2) => ...

During type checking this will expand as

	MacroPat.unapply(<unapply-dummy>)

Meaning that macro can’t see inner1 and inner2 in it’s macroApplication.
To circumvent this we attach that info as an attachment to the dummy.
This commit performs a number of preliminary refactoring needed to
implement unliftable:

1. Replace previous inheritance-heavy implementation of Holes with
   similar but much simpler one. Holes are now split into two different
   categories: ApplyHole and UnapplyHole which correspondingly represent
   information about holes in construction and deconstruction quasiquotes.

2. Make Placeholders extract holes rather than their field values. This
   is required to be able to get additional mode-specific information
   from holes (e.g. only ApplyHoles have types).

3. Bring ApplyReifier & UnapplyReifier even closer to the future where
   there is just a single base Reifier with mode parameter.

Along the way a few bugs were fixed:

1. SI-7980 SI-7996 fail with nice error on bottom types splices

2. Use asSeenFrom instead of typeArguments in parseCardinality.
   This fixes a crash if T <:< Iterable[Tree] but does not itself
   have any type arguments.

3. Fix spurious error message on splicing of Lists through Liftable[List[T]]
Unliftable is a type class similar to existing Liftable that lets
users to extract custom data types out of trees with the help of
straightforward type ascription syntax:

  val q“foo.bar(${baz: Baz})” = ...

This will use Unliftable[Baz] to extract custom data type Baz out of
a tree nested inside of the another tree. A simpler example would be
extracting of constant values:

  val q”${x: Int} + ${y: Int}” = q”1 + 2”
@scala-jenkins
Copy link

(kitty-note-to-self: ignore 29800547)
🐱 Roger! Rebuilding pr-scala for f0ce00d. 🚨

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 29803695)
🐱 Roger! Rebuilding pr-scala for f52b32d. 🚨

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 29804287)
🐱 Roger! Rebuilding pr-scala for 125a349. 🚨

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 29804295)
🐱 Roger! Rebuilding pr-scala for 0ea7c94. 🚨

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 29804311)
🐱 Roger! Rebuilding pr-scala for cd9c49f. 🚨

@densh
Copy link
Contributor Author

densh commented Dec 4, 2013

Wow, it finally builds as it should. Last commit in the list is actually green too if you click on it.

@retronym
Copy link
Member

retronym commented Dec 6, 2013

LGTM. Well constructed sequence of commits.

In addition to the my small suggestions above (which could be submitted in a followup, could you please expand the documentation: http://docs.scala-lang.org/overviews/macros/quasiquotes.html ?

@xeno-by
Copy link
Contributor

xeno-by commented Dec 8, 2013

@retronym @densh See my comments above

densh added 5 commits December 9, 2013 17:41
1. Use (x1, x2): (T1, T2) instead of (x1: T1, x2: T2)

2. More detailed error message for improper function argument

3. Fix typo

4. Completely remove LiftableClass symbol from definitions
@densh
Copy link
Contributor Author

densh commented Dec 10, 2013

PLS REBUILD/pr-scala@e9efd08

@densh
Copy link
Contributor Author

densh commented Dec 10, 2013

PLS REBUILD/pr-scala@a83fe95

@densh
Copy link
Contributor Author

densh commented Dec 10, 2013

PLS REBUILD/pr-scala@cf19f82

@densh
Copy link
Contributor Author

densh commented Dec 10, 2013

PLS REBUILD/pr-scala@0047a96

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 30215500)
🐱 Roger! Rebuilding pr-scala for e9efd08. 🚨

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 30215522)
🐱 Roger! Rebuilding pr-scala for a83fe95. 🚨

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 30215533)
🐱 Roger! Rebuilding pr-scala for cf19f82. 🚨

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 30215543)
🐱 Roger! Rebuilding pr-scala for 0047a96. 🚨

@densh
Copy link
Contributor Author

densh commented Dec 10, 2013

IDE integration really hates this pull request.

@retronym
Copy link
Member

rebase it on master.

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