Attributes on interfaces: implementation issues

Attributes on interfaces are currently allowed (and IMO a sensible extension), but their implementation is broken, due to an implementation quirk (see tests coll/interface_03p and coll/interface_04p):

synthesised attributes and collection attributes from interfaces are deep-cloned (deepCopy()) for each implementing ASTDecl (TypeDecl, even though I am not sure why) and placed in collDecls() and synDecls() together with deep clones of non-interface syn and coll attributes.
As a side effect of this, decl() for each cloned SynEq and CollEq points to the local clone of the corresponding SynDecl resp. CollDecl.
This behaviour is different from attributes inherited from superclasses, which have one declaration that all implementations reference via their decl().
As a consequence of this behaviour, CollDecl.weaveCollectionAttribute() will not notice if some ASTNode
has specific contributions and generate survey code twice (since ASTDecl.hasCollEq(CollDecl) compares the original CollDecl against its deep clone CollDecl).
I see similar behaviour with syn attributes and assume that it is a similar issue.
I have not checked inh attributes.

‌

Possible solutions:

The solutions below are for collection attributes; I am assuming that analogous approaches would work for other attribute kinds (syn / inh), as needed.

An easy solution for the currently known bugs (two bugs currently marked as known-to-be-broken in jastadd-tests) is to consider CollDecl nodes to be equal (for the purposes of hasCollEq) if they have the same collectionId()
A possibly more robust solution is to change how attributes on interfaces are processed, for instance:
1. Split CollDecl into two: A CollPureDecl that is unique for each declaration and never cloned and that serves as result for decl(), and a CollInstance that is cloned. CollPureDecl would handle code generation of pure declarations (including for interfaces), while CollInstance would handle any other code generation.
2. Avoid cloning attribute declarations: From what I understand, the main reason for cloning attribute declarations in collDecls() is to be able to use the inherited attribute hostClass() to change part of the behaviour of these classes. As an alternative, this attribute could be passed in as a parameter during relevant operations, which would then execute on the original (non-cloned) AST parts. Since hostClass() is used in quite a few places, this might be a bit clunky in practice.
3. Use some other form of indirection to be able to tell cloned CollDecl nodes whether they are “the original” or a clone. For instance, collDecls() might place cloned CollDecl nodes under an InheritedCollDecl node that uses an inherited attribute to override their behaviour. It could use a setter to tell InheritedCollDecl where the original node was, and then delegate most attributes to the CollDecl child.
4. CollDecl and friends could carry the knowledge that they have been deep-cloned (and a reference to the original CollDecl), updated by a setter after the CollDecl is cloned.

Solution 1 is simplest but may not be robust, in the sense that it doesn’t answer the question of what a CollDecl actually represents.
Solution 2.1 (2a) maintains the invariant that each attribute declaration has a single representative AST node; the downside is that one user input now translates into two different node types.
Solution 2.2 (2b) maintains a smaller AST and is more explicit, but requires more complex parameter wrangling.
Solution 2.3 (2c) introduces a node whose only role is to provide information to its unique child; this approach would require relatively few changes to the AST, but we would need an InheritedSynDecl node, too, and perhaps an InheritedInhDecl node.
Solution 2.4 (2d) is the least “clean”, in that it uses mutable state on a nontrivial AST node, though the implicit contract would be that the state is only set right after construction or not at all.

I would lean towards 2.4 (2d), but I am not very familiar with JastAdd’s internals or the design rationale behind the AST; any thoughts / recommendations?

Comments (4)