This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH/RFA] spec functions
On Sat, Nov 02, 2002 at 08:42:48AM -0800, Jason R Thorpe wrote:
> On Fri, Nov 01, 2002 at 10:39:23PM -0500, Daniel Jacobowitz wrote:
>
> > > + /* List of static spec functions. */
> > > +
> > > + #define INIT_STATIC_SPEC_FUNCTION(name, func) \
> > > + { NULL, name, func }
> > > +
> > > + static struct spec_function static_spec_functions[] =
> > > + {
> > > + INIT_STATIC_SPEC_FUNCTION ("if-exists", if_exists_spec_function),
> > > + };
> >
> > I'm curious, why the macro? A NULL isn't all that ugly...
>
> Style, mostly. Doesn't really matter to me either way. In the event
> that the table becomes large one day, I wonder which will be easier
> to read...
Well, the NULL is shorter :)
> > > + for (sf = spec_functions; sf != NULL; sf = sf->next)
> > > + fatal ("duplicate `%s' spec function", name);
> >
> > Surely you've lost a line or two! Shouldn't there be a strcmp?
>
> Uh, yah, oops.
>
> > > + processing_spec_function++;
> >
> > Why do you disallow spec functions in spec function arguments? It's
> > not terribly important, but it's unintuitive (and I didn't see it in
> > the docs either).
>
> Um.. That should work fine.. oh, but it doesn't (I just tried nesting
> two %:if-exists() calls) ... there's a small problem with the arg splitting
> in that case (it gets an extra empty argument). Ok, I'll fix that.
>
> The processing_spec_function indicator is basically to make sure that
> execute() doesn't do anything (except abort :-)... that's all.
I don't really understand why that bit is necessary either... there's
probably a good reason though :)
> > Might want to comment that the usual use involves %s, and that it
> > doesn't actually do any expansion - it wasn't clear from the
> > comments/docs.
>
> Yah, I'll clarify that a bit.
>
> Thanks for the feedback -- I'll post an updated patch shortly.
Thanks!
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer