Fault tolerant transformers #383
Conversation
…nguish between missing and failing transformations
…ceholder node annotations
| (param.type.classifier as KClass<*>).isSubclassOf(Node::class) -> | ||
| (param.type.classifier as KClass<out Node>).dummyInstance() | ||
| param.type == String::class.createType() -> "DUMMY" | ||
| param.type.classifier == ReferenceByName::class -> ReferenceByName<PossiblyNamed>("UNKNOWN") |
There was a problem hiding this comment.
This is unlikely, but the reference could be "resolved" to a real element called "UNKNOWN". Is it possible to have a ReferenceByName subclass that can't possibly be resolved?
There was a problem hiding this comment.
I see your point, but I would aim to solve it by never trying to resolve references belonging to "dummy nodes" or "dummy trees". So I would consider:
- Adding a flag
val Node.isDummythat looks at the origin parameter and possibly also to the parent - Adding a flag in
walkand similar methods, to give the option to skip dummy nodes - When resolving references use the
walkmethod using the flag that skip dummy nodes
In this way we never attempt to solve references. Alternatively we can also create subclass of ReferenceByName that explode if you attempt to mark it as solved.
What do you think?
There was a problem hiding this comment.
I'm not a fan of modifying walk, but the symbol resolver could apply a filter to the nodes it walks to exclude dummy ones, it makes sense
| // We either find complex logic to find the ones that aren't or just pick one randomly. | ||
| // Eventually we will build a tree | ||
| val r = Random.Default | ||
| subclasses[r.nextInt(subclasses.size)].toInstantiableType() |
There was a problem hiding this comment.
I'm not a fan of random here because parsing the same file twice will likely give different results, which I think is undesirable (even if we can say that the behavior is unspecified for code that we don't yet support).
I would also consider a mechanism (marker interface or annotation) with which we can designate a specific subclass as the default/fallback node, because in my experience it's not uncommon to have classes such as UnsupportedStatement or ExpressionError already in use.
There was a problem hiding this comment.
That is a good point. My goal is to avoid that we pick always the wrong one as ArrayType(ArrayType(ArrayType(...))), but perhaps what we can do is to pick a pseudo-random number, that actually depends on the depth of the "DummyNode". I will work on that. Adding an annotation could make our life easier here, but it would require extra work when building the language, so I would lean toward doing more work on this code to have to do less work on all the languages
There was a problem hiding this comment.
I see. Anyway I guess this only applies to 1.5 because in 1.6 we'll have machine-generated error nodes to use.
core/src/main/kotlin/com/strumenta/kolasu/transformation/Transformation.kt
Show resolved
Hide resolved
|
Thank you @alessiostalla for the comments, I think the PR is now improved |
When we implement transformers we may have the case in which the transformation is missing and the case in which it is failing. It may be failing because we cover a certain subset of cases. For example, when transforming from RPG to Java, we may cover a field declaration if the field is a string or a number, but not if it is of type pointer. In these cases we may want to still produce a partial transformation, treating the failure similarly to what we do with missing transformations. So expand the strategy for missing transformation to cover also failing transformations.
In addition to this: