Quoting Bert Wesarg <[EMAIL PROTECTED]>:
> On Tue, Sep 23, 2008 at 00:15, Tony Balinski <[EMAIL PROTECTED]> wrote:
> > Quoting Bert Wesarg <[EMAIL PROTECTED]>:
> >
> >> This is my proposal for fixing several issues with the handling of
> compiled
> >> macro programs.
> > ...
> >> If there are no objections, I commit this by the end of the week, plus
> my
> >> proposal B for SF: BUG#2074318, from here:
> >
> > setupFrame() doesn't check the remaining space in the stack to see if the
> > call frame will fit. In my latest stuff, I just folded all stack frame
> > building into a routine, callSubroutineFromSymbol(), which uses the
> current
> > context so that the PUSH macros etc will do the check (I added a
> PUSH_CHECK
> > macro to allow checking for the function linkage space in one go). For
> > rewindFrame() checking is less important: all should be correct already.
>
> I added these check macros to my 'interpre.c macro cleanup' patch.
>
> The 'problem' (it isn't really a problem only the current situation)
> is, that the POP/PUSH macros all work on the global StackP variable,
> which make these useless for setupFrame(). I totally agree that any
> code that setup the next function frame should at lest check the stack
> size. Part of the current problem (and you have realized this long
> before:
>
> +/* TODO: this function should really return a status (if fails PUSH_CHECK)
> */
> void RunMacroAsSubrCall(Program *prog)
>
> ) is that not all frame setup sites handle a stack error.
Not quite the same issue, and not much of a problem: RunMacroAsSubrCall
only ever runs new macros, where the frame size is limited. In this case stack
overflow shouldn't really be an issue. But it is annoying!
> > Why not have it take a RestartData *context argument BTW?
> setupFrame() and rewindFrame() are RestartData agnostic, so I decided
> that rewindStack() should be agnostic too. Despite the fact, that it
> is currently only called from FreeRestartData().
>
> Ohh, I see that you asked for rewindFrame(), thats RestartData
> agnostic, because it is also called from returnValOrNone(), which
> hasn't a RestartData.
I actually contemplated removing PC, FrameP, StackP etc and doing
everything with RestartData contexts.
> >
> > setupFrame() also checks for a negative args count. That's an extension I
> > added for the named arguments/arguments array stuff. You shouldn't need
> it in
> > the code base (yet!).
> I know this but they don't collide.
Indeed.
> What collides is the haveNamedArg,
> which is flagged by your negative nArgs. But if I call setupFrame()
> you have removed your negative flag from nArgs already. I could also
> use a flag parameter and combine the passProg and argsOnStack flags
> into this. And in your anonArray patch you add a new flag for
> haveNamedArg.
>
> > Other than that, having these two functions defined close together is a
> very
> > sound idea. And it's good that the same operations aren't repeated 3
> times
> > over (as in the current source base).
> >
> > As for your 2074318 stuff, why not?
> Because I posted two proposals, proposal A would just bail out if more
> that 9 arguments are passed.
Oops, that was a rhetorical question. I really meant: yes!
> >
> > All in all, I'm for it. Does anyone else have anything to say? This would
> be
> > for the v5.6 ("L'Arlesienne") version, I suppose!
> Hmm, my french was never good, but it is probably not a positive word.
It's like Godot in English: someone who should be there but never shows up!
Tony
--
NEdit Develop mailing list - [email protected]
http://www.nedit.org/mailman/listinfo/develop
