Skip to content
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

Add toExprOfTuple in scala.quoted #7037

Merged
merged 3 commits into from
Aug 19, 2019

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki nicolasstucki self-assigned this Aug 13, 2019
@milessabin
Copy link
Contributor

LGTM!

I think we also need something to construct a tuple type given a Seq[Type].

@nicolasstucki
Copy link
Contributor Author

Still need to add at least 23 test

@nicolasstucki
Copy link
Contributor Author

What would be the signature of the Seq[Type]?

@milessabin
Copy link
Contributor

What would be the signature of the Seq[Type]?

How about,

def tupleType(tpes: Seq[reflect.Type]): reflect.Type

Or should that be quoted.Type? I have to be honest, I don't yet have a clear mental model of when I should be working with quoted vs tasty.reflect types.

@milessabin
Copy link
Contributor

milessabin commented Aug 14, 2019

The context is, I want to be able to generate a trait implementation of the form,

class FooImpl extends Foo {
  type ElemTypes = <tuple type here>
  val Elems: ElemTypes = <tuple term here>
}

where the tuple elements are typed tasty trees (from an annots call) and the tuple element types are the tasty types recovered from those trees.

I'm open to changing the approach so long as I can produce a term of that form from whatever I get back from annots.

@nicolasstucki
Copy link
Contributor Author

That type for tuple is completely unrelated to this PR. I will add work on It in a different PR.

@milessabin
Copy link
Contributor

That type for tuple is completely unrelated to this PR. I will add work on It in a different PR.

Awesome ... thanks. And let me know if there's anything I can do to help here.

@nicolasstucki nicolasstucki marked this pull request as ready for review August 15, 2019 12:46
@nicolasstucki
Copy link
Contributor Author

That type for tuple is completely unrelated to this PR. I will add work on It in a different PR.

Awesome ... thanks. And let me know if there's anything I can do to help here.

Could you make minimal full example/test where the only thing missing is the type of the tuple?

@anatoliykmetyuk
Copy link
Contributor

Possibly related: #7047 though it lifts tuples, not sequences, and is currently blocked.

Test does not compile with type annotations uncommented.
@milessabin
Copy link
Contributor

@nicolasstucki I'm not sure if that's exactly what you meant by "Could you make minimal full example/test where the only thing missing is the type of the tuple?", but the test I've just added doesn't compile if you uncomment the type annotations, failing with,

-- Error: tests/run/quote-toExprOfTuple/Test_2.scala:2:19 ----------------------
2 |  val t2 = Macro.t2(23, "foo")
  |           ^^^^^^^^^^^^^^^^^^^
  |Exception occurred while executing macro expansion.
  |scala.tasty.reflect.ExprCastError: Expr: {
  |  Tuple2.apply[Any, Any](
  |    {
  |      {
  |        23
  |      }
  |    }
  |  , 
  |    {
  |      {
  |        "foo"
  |      }
  |    }
  |  )
  |}
  |did not conform to type: (Int, String)
  |
  |     at dotty.tools.dotc.tastyreflect.ReflectionCompilerInterface.QuotedExpr_cast(ReflectionCompilerInterface.scala:1767)
  |     at dotty.tools.dotc.tastyreflect.ReflectionCompilerInterface.QuotedExpr_cast(ReflectionCompilerInterface.scala:1759)
  |     at scala.tasty.reflect.QuotedOps$QuotedExprAPI.cast(QuotedOps.scala:13)
  |     at Macro$.impl2(Macro_1.scala:12)
  |
  | This location is in code that was inlined at Test_2.scala:2
one error found
[error] Nonzero exit code returned from runner: 1
[error] (dotty-compiler-bootstrapped / Compile / runMain) Nonzero exit code returned from runner: 1
[error] Total time: 3 s, completed 15-Aug-2019 16:05:29
sbt:dotty>

@@ -0,0 +1,14 @@
import scala.quoted._

object Macro {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test should be in tests/run-macros

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 fixed it. It was probably a bad interaction with the non bootstrapped compiler.

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM

@nicolasstucki nicolasstucki merged commit cea5a62 into scala:master Aug 19, 2019
@nicolasstucki nicolasstucki deleted the add-seq-to-expr-tuple branch August 19, 2019 11:28
@milessabin
Copy link
Contributor

I'm a bit confused ... the test I added didn't compile with the type annotation uncommented, which looked wrong to me.

Was that only the case because the test was being compiled with the wrong flags? If that's so then the test should have the type annotations restored, surely?

@nicolasstucki
Copy link
Contributor Author

I will have a second look at it

@nicolasstucki
Copy link
Contributor Author

Followed by #7076

@anatoliykmetyuk anatoliykmetyuk added this to the 0.18 Tech Preview milestone Aug 28, 2019
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.

None yet

4 participants