I've looked over over the diffs between trunk and leo-ctx5, and here
are my notes.
{{ OVERALL }}
A significant improvement. Good work, y'all.
{{ USER-VISIBLE }}
* optional parameter interface: ":opt_count" -> ":opt_flag"
+ As some of you may recall, ":opt_count" semantics don't work if
we ever support named parameters ... and given that p6l may be
moving toward making P6 named parameters less insane[*], this is
not impossible for Parrot 1.1 or something.
So ":opt_count" should be replaced with ":opt_flag", which sets
its target register to a boolean indicating whether the
immediately preceding parameter was passed or not, rather than a
count of optionals present.
If you want to do this after the merge, OK, but please don't
release with ":opt_count".
I also had some idea of how to spell this more compactly, but
":opt_flag" has to work first, so let's start with that.
This needs to be fixed before merge, to minimize PIR language
shift for users.
* subroutine attribute spelling: "method" -> ":method"
+ Some time ago, in a conversation perhaps lost to time, I pointed
to ':method', not 'method', as the way method tags should look
on sub definitions. No commas, just ':method' and, at some
point (if not yet), ':multi':
.sub "foo" :multi(_,int) :method
Of course, for the time being, "@MULTI" is a synonym for ":multi".
* src/embed.c : setup_argv() - setting P5
+ Looks like this function is still setting P5. Hm. (?)
* src/inter_run.c : runops_args() - argument limit
+ Looks like this function fails if someone passes more than eight
parameters. Why? Buffer malloc is cheap.
* DELETED opcode(s) can die
+ The changes to the invoke opcodes are binary-incompatible, which
I have NO PROBLEM with, but as long as we're breaking things, you
might as well get rid of "DELETED" opcodes, like "DELETED_invoke".
* Tests may not use literal get/set flags
+ The values of the flags for get_params/set_args/etc. should not
escape in numeric form, even to the self-tests. If they're out
there in symbolic form, fine. Just not as numbers. I want to
be able to change those numbers without causing errors; and more
to the point, I want to make sure that the tests that use a given
feature fail if that feature is removed.
The tests that use literal numeric get/set flag values should be
using PIR instead, I suppose. Or the syntax should be extended
to allow symbolic values instead of numbers. Whatever. The
point is that this, from pmc/mmd.t, should not exist:
get_params "(0,0,0x20,0x40)", P5, I5, P6, I7
{{ INTERNAL }}
* struct Parrot_Context
+ "ref_count"
Why are we going there? Refcounting bad, reference observation
via GC good, right? I may have totally missed the point of this.
* classes/bound_nci.pmc
+ The invoke() method is still setting P2, but P2 isn't supposed
to be special any more.
Update: On IRC, Leo describes bound_nci as "b0rken"; OK, but
it'd be better to nuke that code and replace it with a die and
comment as to what should happen.
* include/parrot/interpreter.h
+ There are two different structures with "current_object".
Is this OK, or a surprise bug, a temporary hack, or ... ?
* imcc/pbc.c
+ A comment has been removed from verify_signature, but it still
applies, and should be restored. (Please don't remove 'todo'
comments without discussion.)
- * Actually it doesn't verify anything, but rather, it _corrects_ it,
- * silently. Which needs to change. XXX CHS
* pcc/get/args.c
+ I think this comment speaks for itself:
+ strcat(buf, s); /* XXX check avail len */
That's a security/coredump problem. It can't wait until later.
* documentation
+ I'm not asking you to go back and add docs for everything -- not
that I'd mind! -- but when you touch a function so that its API
changes (and creating a subroutine where none had existed counts
as an API change), the function needs to be (re)documented.
It's just a matter of making sure everyone knows what it's
supposed to do and how to call it.
[*] I think we can all agree that it would be impossible to make them
_more_ insane.
--
Chip Salzenberg <[EMAIL PROTECTED]>
