Add constructors for TypeOrBounds that are not refs or PolyType #7961
Conversation
dottybot
left a comment
dottybot
left a comment
There was a problem hiding this comment.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Have an awesome day! ☀️
|
Review by @nicolasstucki |
|
Seems there's some kind of build issue... community build failed with In any case, have a good week-end. |
|
I restarted the tests and it passed. |
compiler/src/dotty/tools/dotc/tastyreflect/ReflectionCompilerInterface.scala
Outdated
Show resolved
Hide resolved
| ).seal.asInstanceOf[quoted.Type[Any]] | ||
|
|
||
| '{ | ||
| val x1 : ${x1T} = 1 |
There was a problem hiding this comment.
This is only testing that 1 is a subtype of x1T but not the other way around.
I propose to try a more direct approach with assetrions and without asInstanceOf like:
val x1T = ConstantType(Constant(1))
val x2T = OrType(ConstantType(Constant(1)), ConstantType(Constant(2)))
val x3T = AndType(ConstantType(Constant(3)), typeOf[Any])
val x4Lam =
TypeLambda(
List("A","B"),
_ => List(TypeBounds(typeOf[Nothing], typeOf[Any]), TypeBounds(typeOf[Nothing], typeOf[Any])),
(tl : TypeLambda) => tl.param(1))
val x5T =
Refinement(
typeOf[RefineMe],
"T",
TypeBounds(typeOf[Int], typeOf[Int]))
...
assert(x1T =:= '[1].unseal.tpe)
assert(x2T =:= '[1 | 2].unseal.tpe)
assert(x3T =:= '[1 & Any].unseal.tpe)
assert(x4T =:= '[[A, B] =>> B].unseal.tpe)
assert(x5T =:= '[RefineMe { type T = Int }].unseal.tpe)
...Then you could also move the test to tests/run-macros
There was a problem hiding this comment.
Good point. I've updated the tests to do this (code push coming soon).
I don't understand "move the test to tests/run-macros" though. That is already where the test is?
There was a problem hiding this comment.
I meant tests/pos-macros. Sorry.
There was a problem hiding this comment.
But it is ok like this
| */ | ||
| def MatchCaseType(given Context): Type = { | ||
| import scala.internal.MatchCase | ||
| Type(classOf[MatchCase[_,_]]) |
There was a problem hiding this comment.
This is too hacky. In the future, we won't have access to the classOf[MatchCase[_, _]]. This should probably be in the scala.internal interface (CompilerInterface).
There was a problem hiding this comment.
This is interesting. Even in the tests, I had to use Type(classOf[List[_]]) in order to get the type of List.
Actually writing typeOf[List] gives me missing type parameter(s) for List. Writing typeOf[List[_]] satisfies Dotty but gives me AppliedType(List, TypeBounds(Nothing,Any)), which is not helpful.
Is this a bug or am I doing it wrong?
There was a problem hiding this comment.
(leaving is as-is for now since it makes more sense to change this and the tests at the same time once we figure out the above question)
Constructors added: - TypeBounds - ConstantType - AnnotatedType - AndType - OrType - TypeLambda - MatchType All these types are tested by using `=:=` to compare them to equivalent type quotes. Additional tests are included for constructors that were not added `Refinement` and `AppliedType` because they are related and would have been added were they not originally present. Aside from constructors, `TypeLambda.param(Int)` was added in order to refer to the parameters of a `TypeLambda` during construction. An accessor for `internal.MatchCase[_,_]` was also added to make the `MatchType` constructor useable in practice. Things like `TypeRef`, `TermRef`, `ThisType` are not included because I could not think of a use for them that is not better (and more safely) served by less direct methods as I used in my test code. `PolyType` is not included because to test that we would need need a way to synthesize `DefDef` declarations, which is not currently supported. `ByNameType` (or, `ExprType`) is also not testable for similar reasons.
fhackett
force-pushed
the
fhackett-work-on-6280
branch
from
72573e7 to
5b75df9
Compare
|
Thank you @fhackett |
3 participants
This PR addresses #6280, at least in part. Review by @nicolasstucki
Constructors added:
All these types are tested by splicing them into the types of
vals. Additional tests are included for constructors that were not addedRefinementandAppliedTypebecause they are related and would have been added were they not originally present.Aside from constructors,
TypeLambda.param(Int)was added in order to refer to the parameters of aTypeLambdaduring construction.An accessor for
internal.MatchCase[_,_]was also added to make theMatchTypeconstructor useable in practice.Things like
TypeRef,TermRef,ThisTypeare not included because I could not think of a use for them that is not better (and more safely) served by less direct methods as I used in my test code.PolyTypeis not included because to test that we would need need a way to synthesizeDefDefdeclarations, which is not currently supported.ByNameType(or,ExprType) is also not testable for similar reasons.