[Libre-soc-bugs] [Bug 1094] insndb instruction database visitor-walker is needed

[bugzilla-daemon at libre-soc.org]

https://bugs.libre-soc.org/show_bug.cgi?id=1094

--- Comment #35 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Dmitry Selyutin from comment #32)

> Now that you mentioned this, I thought I should provide a separate .visit
> method for handling these, like Database and Record classes have. These
> should come handy for C code generation too.

i believe you may be getting to the point where my suggestion to have
the visitor-walking be much more part of each DB-class itself makes 0
sense.

in this way *at the class definition* it describes its "sub-things"

Database :

* declares to all Vistors that it has 32-bit instructions
* declares to all Vistors that it has SVP64 instructions
* declares to all Visitors that it has Fields and Forms


(in the python-html DOM/SAX visitor these are all identified by
the function names).



Record:

* declares that it has {CSV-properties}


and the Visitors walk that *entire lot* looking for corresponding
function/object-names and calling them with the data.

(In reply to Dmitry Selyutin from comment #33)

> Introduced "visitable" extra object and the corresponding handler. This,
> however, got tricky, since visitors had to track what exact instructions
> they visit. 

yes they do.  it is never easy to do visitors because the *declaration*
of the database defines how to walk it, but that declaration never
contains the data.

(we are back to things like BNF, SemanticWeb, XSLT etc *shudder*)



> Please check the recent commits, I'd like have some ideas on
> improvements to make it less messy.


https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=7ffad8f7202


 class BaseVisitor(Visitor):
-    def __init__(self, **_):
-        pass
+    def __init__(self, **arguments):
+        self.__arguments = types.MappingProxyType(arguments)
+        self.__current_db = None
+        self.__current_record = None
+        self.__current_extra = None
+        return super().__init__()

ahhh.... i need to check: a Vistor *itself* may store
state-information, but *not all visitors need to do so*.

the *walking* aspect ("how to walk") is intended to be utterly
separate from the "tracking" aspect, and for many Visitor-instances
they hold an open file-handle and the visited-functions simply
call "write" on it, using their arguments.

so the *walker* simply ends up with stuff on the stack.  lots of it.
as it drills down into more child-data.

if the Vistor *actually needed* any prior state (a parent),
it is the responsibility *of that Visitor instance on its begin function*
to make a note of it.


so i need to check: are you intending *ALL* Visitors to critically
rely on the existence of self.__current_db and so on?

because i am concerned that all three of these have been added

+        self.__current_db = None
+        self.__current_record = None
+        self.__current_extra = None

record is part of *Record* visiting.

extra is part of *Extra* visiting.

there should not even really be **arguments.
(and certainly no use of python types module)

python's standard module "types" - types.MappingProxyType(arguments)
really should not be used.

-- 
You are receiving this mail because:
You are on the CC list for the bug.