chromatic wrote:
+=item C<PMC* inspect_str(STRING* what)>
The star should go on the variable name (at least for incoming parameters).
That way we avoid the problem of:
char* foo, bar; /* oops, bar isn't a pointer! */
Yeah, I know that convention and try to stick with it. I copy-pasted
those signatures from src/vtable.tbl, which doesn't adhere to this rule,
then didn't spot it to change it.
The mem_sys_allocate_zeroed_typed() is slightly nicer here, and it's C++
friendly.
Cool...not sure that existed last time I did any serious Parrot
internals hacking. Someone beat me to changing that one, though. :-)
I'd hoist the assignment of pc up to its declaration.
Done too.
+ if (PARROT_ARG_SLURPY_ARRAY_ISSET(sig_item)){
+ if (PARROT_ARG_NAME_ISSET(sig_item))
+ sub->arg_info->named_slurpy = 1;
+ else
+ sub->arg_info->pos_slurpy = 1;
+ }
+ else if (PARROT_ARG_OPTIONAL_ISSET(sig_item)) {
+ if (PARROT_ARG_NAME_ISSET(sig_item))
+ sub->arg_info->named_optional++;
+ else
+ sub->arg_info->pos_optional++;
+ }
+ else if (!PARROT_ARG_OPT_FLAG_ISSET(sig_item)) {
+ if (PARROT_ARG_NAME_ISSET(sig_item))
+ sub->arg_info->named_required++;
+ else
+ sub->arg_info->pos_required++;
+ }
This logic confuses me slightly. What's the difference between OPTIONAL and
OPT_FLAG?
Please see PDD03.
+ /* Return the argument information that was requested. */
+ if (string_equal(interp, what, CONST_STRING(interp,
"pos_required")) == 0) {
The STR_EQ macro simplifies this slightly.
Can't find a reference to that macro anywhere...I did find STREQ, but I
only see it being used on char *'s and not on STRING *'s.
The return here is unreachable.
I know that. Compilers don't and emit warnings.
You can probably get rid of the else clause and one level of nesting and put the exception as the last part of the code.
That way it may be clearer that there's one and only one good way out of this function.
Goes nicely on the end of the else block.
Done in r25588.
Thanks,
Jonathan
