use erased extensions to implement operatorExtension API#1295
use erased extensions to implement operatorExtension API#1295
Conversation
|
Sorry but I don't think this is a big enough improvement from the status quo to justify adding more complexity to the generated code. Anyone who hasn't spent hours on this topic and is a also scala expert will have a hard time understand why we need these newly introduced covariant type parameters, which are even unused, so at first sight they look like bogus. As you correctly point out, there's a bug in ODB where multiple NodeRefs for the same underlying NodeDb can lead to issues. This looks like it can be fixed in ODB, and the only reason we're hesitating to fix it is that in doesn't play any role in practice. I'm guessing you're so persuasive on this topic because this "constrains what we can do in an odb follow-up", and that's fair enough. If the odb-followup is so good (fast, memory-efficient, ...) that we want to use it, then a change like this is not a deal-breaker. As it stands, while we're using ODB, I wouldn't want this change (especially the one for odb-codegen). How about you carry on with the ODB-followup prototype and assume that we can (eventually) merge this PR. But for now we don't merge it. |
This uses erased extensions to implement the operatorExtension API. It is an alternative to #1293 and relies on ShiftLeftSecurity/overflowdb-codegen#86
I guess we should move discussion of implementation details of the erased extensions (e.g. naming bikeshedding) to ShiftLeftSecurity/overflowdb-codegen#86 and move general discussion of this approach here.
So: Are we happy with erased extensions? The idea of erased extensions is that we add an unused covariant generic type parameter to our classes, and make a type-alias
type oldClassName = newClassName[Any]. The parametric type gets erased, i.e. does not affect runtime behavior.However, it participates in scala type inference and implicit conversions. Hence we can use it for DSL-style things like the operator extensions.
Since the operator extensions are supposed to be a simple example, I compressed it down to two files (make it easier for newcomers to grok the entire package).
Note that the erased extensions cannot be runtime-checked -- i.e. there can be no runtime type errors or
x.isInstanceOf[Assignmant]checks.Generally, I see three approaches to the underlying issue with subclassing the NodeRef classes:
cc @mpollmeier @fabsx00 @ml86 and @hubertp