Opened 6 years ago
Closed 19 months ago
#128 closed enhancement (fixed)
Rearchitect TupleType
Reported by: | a3moss | Owned by: | |
---|---|---|---|
Priority: | minor | Component: | cfa-cc |
Version: | 1.0 | Keywords: | |
Cc: |
Description
Rob left some comments on the design of the TupleType
AST node suggesting some different design directions (see src/SynTree/TupleType.cc
or src/AST/Type.cpp
). The gist is that the relationship between the types
and members
fields is non-trivial, and could be factored out more cleanly.
Comment verbatim below:
This is very awkward.
TupleType
should contain objects so that members can
be named, but if they don't have an initializer node then they end up
getting constructors, which end up being inserted causing problems. This
happens because the object decls have to be visited so that their types are
kept in sync with the types list here. Ultimately, the types list here
should be eliminated and perhaps replaced with a list-view of the object
types list, but I digress. The temporary solution here is to make a
ListInit
withmaybeConstructed = false
, that way even when the object is
visited, it is never constructed. Ultimately, a better solution might be
either:
a) to separate
TupleType
from its declarations, intoTupleDecl
and
Tuple{Inst?}Type
, alaStructDecl
andStructInstType
b) separate initializer nodes better, e.g. add a
MaybeConstructed
node
that is replaced bygenInit
, rather than what currently exists in abool
Possibly through waves of refactoring there was actually only one use of the members field left. I changed that one use (in CurrentObject?) so was not a use and then removed the field. This only applies to the new ast (so
src/AST/Type.cpp
and notsrc/SynTree/Type.cc
) as the old version can just stay as is until we remove it.This does not address any of the possible reworks to the initializer node organization. If there are other reasons to make those changes a new issues should be created.