[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 #200 from Dmitry Selyutin <ghostmansd at gmail.com> ---
(In reply to Luke Kenneth Casson Leighton from comment #199)
> (In reply to Dmitry Selyutin from comment #181)
> > I like option 1, because it's short and sweet, and users can just declare
> > method as `method(self, instance, *_)`.
> 
> indeed.  or, it can be detected: functools can find out how many
> arguments there are, whether there are *args and **kwargs.
> we use it already somewhere, in soc or ah no nmigen, for detecting
> if one of the function parameters is "name".

No. This is _incorrect_. You shouldn't do this, because it's completely
unreliable and can break at any time. The best bet would perhaps be inspect.
This is just like people used to detect "unbound methods", then came Guido and
dropped these once and forever. There is no justification for such tricks other
than hacking or avoiding to traverse complex classes hierarchy manually (which
can bring even more problems than inspect). Resorting to such constructs, other
than for self-aware code or truly dynamic construction of some meta object, is
almost certainly a sign of bad architecture.

> this is a serious error, it is a singleton pattern.
> it MUST be a tuple NOT a list.  (basically anything hashable
> is fine, but unhashable is catastrophic. this is a very common
> mistake)

Please don't be so dramatic. You condemn the artificial example, where you
didn't even check the code. If you check it, you'll see that neither default
value is used nor actually this is write-accessed. But this is not the code to
be committed. The walkers, however, use tuples.

> i really do not like path as a mandatory argument for all visitors
> (particularly leaf-nodes and especially the primitives int bool etc).
> if it is optional (see above including *_)  i have no problem with it.

Luke. You don't read the code, _again_. Which part of it says it's mandatory?
Which part binds it to any visitor other than visitor from this example? I
specifically and explicitly told that walkers yield it, and showed it in the
example itself. It's up to users whether they use this information or not in
visitors.

> and if using the name "instance" i recommend correspondingly using
> "instanceid".

Is it reserved or you want to have it parallel with typid? I named it instance
because it's exactly instance of some object, and because we use type(instance)
for a double dispatxh. I'd have used type for typeid, but this one is a
built-in. And, frankly, our typeid is even no longer typeid, because sometimes
it's just callable (like dataclasses.is_dataclass). I couldn't come up with a
good name since I implemented this, ideas are appreciated. :-)

> ahh ok. good explanation.
> 
> > and, yes, ints *need* paths, so you can tell which int you're referring to,
> 
> no, you really don't.  primitive types (bool int float complex str bytes)
> definitely do not need a path...

They do need paths if you want to differentiate which int comes from where.
Jacob gave the nested collections for a reason.

> > like e.g. SelectableInt(123, 45)
> 
> ... but types *derived* from int might: this depends on the implementor,
> if there are any additional properties.
> 
> i.e. SelectableInt is no longer a leaf-node type, it is a tree-type
> due to itself having two leaf-nodes: the value and the bit-width.

This actually depends on how you treat it. In some cases you might opt to treat
it as sequence of bits as well. This might be handy sometimes. But I don't
really believe this should be part of visiting.
This is not the same with fields, though. These are entirely different beasts.
And especially with fields which mark insn fields. They are also integers in
some sense, but they should be walked with exact bits in mind.

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