Re: [perl #118653] [PATCH] Move prototype validation to a separatefunction / stop munging whitespace on parse

On Fri, Jun 28, 2013 at 3:28 AM, Nicholas Clark <nick@ccl4.org> wrote:
> On Thu, Jun 27, 2013 at 11:37:20PM -0700, Tony Cook via RT wrote:
>> On Thu Jun 27 19:43:13 2013, pcm wrote:
>
>> > Also, since the logic for generating warnings is only stored in the
>> > tokenizer itself, if something like set_prototype or a prototype
>> > attribute wanted to do illegalproto warnings, it would have to
>> > duplicate that logic.  Moving the code into a separate function allow
>> > for code reuse for other mechanisms of setting the prototype.
>
> Do the things that you want to follow up with require that this function
> is available to XS code?

Yes - this is prep work for a prototype attribute, so that it behaves
the same as the builtin prototype.  It the attribute is defined
inline, sub foo : prototype($$) {}, it'll be handled directly by
toke.c/op.c, but attributes.xs should support it, too.

I imagine Scalar::Util could make use of it too, although I'm not
proposing that it be made part of set_prototype; I imagine the fact
that set_prototype doesn't do any validation is considered a feature
for those who want to use prototypes for their own purposes.

>
>> > If this patch is accepted, please apply and hold this ticket open;
>> > perldelta will need to be updated, and I'm not sure of how that should
>> > be done, and in the meantime I can base a couple of other illegal
>> > proto warnings fixed off of the new function.
>>
>> This works for me and looks sane, but I'm going to wait for any further
>> comment for a few days.
>>
>> Especially on the public/private part.
>
> As it is structured, I don't think that it is a great API.
>
> I think that a better API would not be to call the warner directly, but
> instead return a description of the error (or NULL, if clean), so that the
> caller can decide how and when to report the error.
>
> That API might (in turn) be wrapped by another (private?) function that
> implements the warning/not warning logic, if that logic would otherwise
> be repeated across the source.
>

Perhaps:

UV validate_proto|NN SV *name|NULLOK SV *proto|bool warn

Right now, perl -we 'sub foo($barb@q);' issues two warnings, one for
the illegal characters, and one for the prototype after '@'.

How about keeping the actual warning within the function, controllable
by a bool parameter, so any clients of this API don't need to worry
about wording or anything, they'll just automatically get messages
which already show up in perldoc.

Then whether a warning was emitted or not, it'll return a bitmask of
which warnings were emitted.  Any value at all means there's something
invalid, and they could check which particular warnings were emitted,
or not.

• [perl #118653] [PATCH] Move prototype validation to a separate function / stop munging whitespace on parse by Tony Cook via RT
• Re: [perl #118653] [PATCH] Move prototype validation to a separatefunction / stop munging whitespace on parse by Nicholas Clark
• Re: [perl #118653] [PATCH] Move prototype validation to a separatefunction / stop munging whitespace on parse by Peter Martini
• Re: [perl #118653] [PATCH] Move prototype validation to a separatefunction / stop munging whitespace on parse by Peter Martini
• [perl #118653] [PATCH] Move prototype validation to a separate function / stop munging whitespace on parse by Father Chrysostomos via RT
• [perl #118653] [PATCH] Move prototype validation to a separate function / stop munging whitespace on parse by Peter Martini
• Re: [perl #118653] [PATCH] Move prototype validation to a separatefunction / stop munging whitespace on parse by Brian Fraser
• Re: [perl #118653] [PATCH] Move prototype validation to a separatefunction / stop munging whitespace on parse by Peter Martini