Finish executeFromVar implementation #976
Conversation
6.0.0 activation
…nterpreter into v6.0.0
…rpreter into v6.0.0
… deserialize test
kushti
changed the title
kushti
changed the title
| * Def done in order to carry on DeserializeContext through stages of compilation intact | ||
| */ | ||
| case class DeserializeContextDef[V <: SType](d: DeserializeContext[V], e: Elem[V]) extends Def[V] { | ||
| override def resultType: Elem[V] = e |
There was a problem hiding this comment.
The connection between SType and Elem[T] is a bit more nuanced.
V <: SType corresponds to Elem[V#WrappedType], in other words, V is a descriptor for V#WrappedType and Elem[T] is a descriptor for T.
There was a problem hiding this comment.
What does that mean ? What do you propose?
There was a problem hiding this comment.
It should be e: Elem[V#WrappedType].
| true | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
Please also add test to LanguageSpecificationV6.
DeserializeContext was not covered there before because it was not supported by compiler, and scripts are required there.
Now it can be added.
There was a problem hiding this comment.
??? There is nothing 6.0 specific or versioned in general in this PR.
There was a problem hiding this comment.
DeserializeContext in not covered in LanguageSpecificationV6, above I explained why. Now it can be covered, but also probably was not used in practice due to lack of ErgoScript support. Now it will be available, so better be covered in the specification.
There was a problem hiding this comment.
Sorry, I do not understand, what do you propose to test ? I mean, which DeserializeContext properties, and why in LanguageSpecificationV6 specifically. All other tests in this specification are testing versioned behavior of features added in 6.0. And ScalaDoc for LanguageSpecificationBase is also proposing to test versioned behavior in LanguageSpecification tests. Have you changed ScalaDoc in some PR maybe ?
There was a problem hiding this comment.
There is no contradiction, every feature can be tested for many versions, but at least for one. Importantly, all components of the interpreter is tested, parsing, compiler stages, tree serialization, evaluation on different inputs, positive and negative cases, expected values are checked. That is why there should be tests for each language feature (operation) here. Versioning is not a central part of it, but important capability.
There was a problem hiding this comment.
Then I guess DeserializeContext should be in LanguageSpecificationV3 along with all features existing since the mainnet launch. But then you need to open an issue at least about doing what you are proposing for all the nodes existing since the launch, to avoid chaotic implementation of 2-3 nodes before it will be forgotten forever. And the question about properties is still not answered. A typical check in LSV6 is like
lazy val bitAnd = newFeature(
{ (x: (Int, Int)) => x._1 & x._2 },
"{ (x: (Int, Int)) => x._1 & x._2 }",
sinceVersion = VersionContext.V6SoftForkVersion)
forAll { x: Int =>
Seq(toAbs).foreach(_.checkEquality(x))
}
forAll { x: (Int, Int) =>
Seq(compareTo, bitOr, bitAnd).foreach(_.checkEquality(x))
}
So a check is testing that for V6 a new features has some behavior. What do you want to check about DeserializeContext in LSV* ?
There was a problem hiding this comment.
Yes, it is an existing feature so can be added to LSV5 as existingFeature(...).
I believe described above what I "want to check" by creating a new property(){...} for DeserializeContext.
Most of the other cases like this are either not known or already recorded as issues. Don't think this PR depend on this.
|
@aslesarenko comments addressed, please make another pass |
This PR contains complete implementation (with tests) of executeFromVar support in the ErgoScript compiler.
Related to #443, #600
TODO: open an issue on executeFromSelfReg