[Scummvm-devel] Decompiler: Documentation feedback
Michael Madsen
michael at birdiesoft.dk
Wed Aug 11 18:33:30 CEST 2010
> -----Original Message-----
> From: Max Horn [mailto:max at quendi.de]
> Sent: Wednesday, August 11, 2010 4:54 PM
> To: Michael Madsen
> Cc: ScummVM devel
> Subject: Re: [Scummvm-devel] Decompiler: Documentation feedback
>
> Hi there,
>
> some more feedback:
>
> * In 3.4, after consulting the source code, it seems to me that
> _addressBase should be in SimpleDisassembler, not Disassembler, if at
> all.
At the very least, it should be available for SimpleDisassembler so you're not restricted to loading your scripts at address 0x0. (Alternatively, you'd have to supply the initial address as a parameter to START_OPCODES - that would also work.)
Whether or not it should be available for Disassembler as well - I suppose you could handle that on your own instead, if you have such a need - but I don't know if that's necessarily better or worse.
> * In 3.4, do we really need to document _disassemblyDone at all? It
> seems like an internal implementation detail to me (In fact, I'd
> consider making it 'private'). Instead, I'd change the documentation
> for disassemble() to more explicitly mention that it will call
> doDisassemble(), but caches the result, so if called again it uses the
> cached data. Hence doDisassemble() is called at most once.
Yes, that probably makes more sense. I'll change it.
>
> * In 3.5.4, in the example it says
> OPCODE_MD(0x14, "add", kBinaryOp, -1, "", "}");
> Why the "}" and not a "+" ? If it is on purpose, there should be an
> explanation.
That's a typo.
>
> * In 4.2, it says literally "stack effect >= 0". I'd rewrite this as
> either "non-negative stack effect", or (less preferable) "stack effect
> $\geq 0$".
Sure, let's go with "non-negative".
>
> * In 5.5, you explain that 0xC0 is handled in a special way. It should
> be made explicit that this simply means that processInst is invoked
> (resp. that the instruction will be treated as described under "Other
> types", regardless of its type).
Agreed.
> * Related to this, the processInst doxygen comment is rather fuzzy. It
> should at least give a brief overview of where it is called, and when /
> for which instructions (kind of a 2-3 line summary of section 5.5). It
> would
...it would what? :) But yes, that comment could probably do with some fleshing out.
Michael
More information about the Scummvm-devel
mailing list