Skip to content

Commit

Permalink
fill_params: fill unused registers with default values if error check…
Browse files Browse the repository at this point in the history 
…ing if off

The default is that error checking is off for call results. The calling.t
test relied on this(!), so it now checks for that the result gets overwritten
with PMCNULL. Also, two bugs in PGE::Exp and Test::More were detected and
fixed because of this change.
  • Loading branch information
@mlschroe
mlschroe committed Nov 2, 2011
1 parent 9e91735 commit 6fe1c97
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 22 deletions.
1 change: 1 addition & 0 deletions compilers/pge/PGE/Exp.pir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,7 @@ tree as a PIR code object that can be compiled.
dec rep
goto %L_2
CODE
.return (1)

.end

Expand Down
4 changes: 2 additions & 2 deletions runtime/parrot/library/Test/More.pir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,11 +686,11 @@ This handles comparisons of array-like and hash-like structures.
goto report_result

compare_array:
( result, diagnosis ) = compare_array( left, right, position )
result = compare_array( left, right, position )
goto report_result

compare_hash:
(result, diagnosis ) = compare_hash( left, right, position )
result = compare_hash( left, right, position )
goto report_result

report_result:
Expand Down
47 changes: 28 additions & 19 deletions src/call/args.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,14 +825,20 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),

GETATTR_FixedIntegerArray_size(interp, raw_sig, param_count);

/* A null call object is fine if there are no arguments and no returns. */
/* Get number of positional args */
if (PMC_IS_NULL(call_object)) {
if (param_count > 0 && err_check)
/* A null call object is fine if there are no arguments and no returns. */
if (param_count == 0)
return;
if (err_check) {
Parrot_ex_throw_from_c_args(interp, NULL,
EXCEPTION_INVALID_OPERATION,
"too few arguments: 0 passed, %d expected", param_count);

return;
}
positional_args = 0;
call_object = NULL; /* so we don't need to use PMC_IS_NULL below */
} else {
GETATTR_CallContext_num_positionals(interp, call_object, positional_args);
}

GETATTR_FixedIntegerArray_int_array(interp, raw_sig, raw_params);
Expand All @@ -846,14 +852,13 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
else {
const INTVAL second_flag = raw_params[param_count - 1];
if (second_flag & PARROT_ARG_CALL_SIG) {
*accessor->pmc(interp, arg_info, param_count - 1) = call_object;
*accessor->pmc(interp, arg_info, param_count - 1) = call_object ? call_object : PMCNULL;
if (param_count == 1)
return;
}
}

/* First iterate over positional args and positional parameters. */
GETATTR_CallContext_num_positionals(interp, call_object, positional_args);

while (param_index < param_count) {
INTVAL param_flags = raw_params[param_index];
Expand All @@ -872,7 +877,7 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
INTVAL num_positionals = positional_args - arg_index;
if (num_positionals < 0)
num_positionals = 0;
if (named_count > 0){
if (named_count > 0) {
if (named_used_list != NULL)
Parrot_hash_destroy(interp, named_used_list);
Parrot_ex_throw_from_c_args(interp, NULL,
Expand Down Expand Up @@ -900,7 +905,7 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
/* Fill a named parameter with a positional argument. */
if (param_flags & PARROT_ARG_NAME) {
STRING *param_name;
if (!(param_flags & PARROT_ARG_STRING)){
if (!(param_flags & PARROT_ARG_STRING)) {
if (named_used_list != NULL)
Parrot_hash_destroy(interp, named_used_list);
Parrot_ex_throw_from_c_args(interp, NULL,
Expand All @@ -925,7 +930,7 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),

Parrot_hash_put(interp, named_used_list, param_name, (void *)1);
}
else if (named_count > 0){
else if (named_count > 0) {
if (named_used_list != NULL)
Parrot_hash_destroy(interp, named_used_list);
Parrot_ex_throw_from_c_args(interp, NULL,
Expand Down Expand Up @@ -1002,7 +1007,7 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
if (param_flags & PARROT_ARG_NAME)
break;

if (err_check){
if (err_check) {
if (named_used_list != NULL)
Parrot_hash_destroy(interp, named_used_list);
Parrot_ex_throw_from_c_args(interp, NULL,
Expand All @@ -1011,6 +1016,8 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
"%d passed, %d (or more) expected",
positional_args, param_index + 1);
}
assign_default_param_value(interp, param_index, param_flags,
arg_info, accessor);
}

/* Go on to next argument and parameter. */
Expand All @@ -1035,7 +1042,7 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
INTVAL param_flags = raw_params[param_index];

/* All remaining parameters must be named. */
if (!(param_flags & PARROT_ARG_NAME)){
if (!(param_flags & PARROT_ARG_NAME)) {
if (named_used_list != NULL)
Parrot_hash_destroy(interp, named_used_list);
Parrot_ex_throw_from_c_args(interp, NULL,
Expand All @@ -1047,9 +1054,10 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
if (param_flags & PARROT_ARG_SLURPY_ARRAY) {
PMC * const collect_named = Parrot_pmc_new(interp,
Parrot_hll_get_ctx_HLL_type(interp, enum_class_Hash));
Hash *h;
Hash *h = NULL;
/* Early exit to avoid vtable call */
GETATTR_CallContext_hash(interp, call_object, h);
if (call_object)
GETATTR_CallContext_hash(interp, call_object, h);

if (h && h->entries) {
/* Named argument iteration. */
Expand Down Expand Up @@ -1078,7 +1086,7 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
}

/* Store the name. */
if (!(param_flags & PARROT_ARG_STRING)){
if (!(param_flags & PARROT_ARG_STRING)) {
if (named_used_list != NULL)
Parrot_hash_destroy(interp, named_used_list);
Parrot_ex_throw_from_c_args(interp, NULL,
Expand All @@ -1097,7 +1105,7 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),

param_flags = raw_params[param_index];

if (VTABLE_exists_keyed_str(interp, call_object, param_name)) {
if (call_object && VTABLE_exists_keyed_str(interp, call_object, param_name)) {

/* Mark the name as used, cannot be filled again. */
if (named_used_list==NULL) /* Only created if needed. */
Expand Down Expand Up @@ -1164,7 +1172,7 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),
/* We don't have an argument for the parameter, and it's not
* optional, so it's an error. */
else {
if (err_check){
if (err_check) {
if (named_used_list != NULL)
Parrot_hash_destroy(interp, named_used_list);
Parrot_ex_throw_from_c_args(interp, NULL,
Expand All @@ -1181,10 +1189,11 @@ fill_params(PARROT_INTERP, ARGMOD_NULLOK(PMC *call_object),

/* Double check that all named arguments were assigned to parameters. */
if (err_check) {
Hash *h;
Hash *h = NULL;
/* Early exit to avoid vtable call */
GETATTR_CallContext_hash(interp, call_object, h);
if (!h || !h->entries){
if (call_object)
GETATTR_CallContext_hash(interp, call_object, h);
if (!h || !h->entries) {
if (named_used_list != NULL)
Parrot_hash_destroy(interp, named_used_list);
return;
Expand Down
7 changes: 6 additions & 1 deletion t/op/calling.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2494,7 +2494,12 @@ pir_output_is( <<'CODE', <<'OUTPUT', "Handling :flat of empty arguments" );
.sub 'main' :main
$P0 = new ['Undef']
($P0) = foo()
unless null $P0 goto L1
$S0 = "PMCNULL"
goto L2
L1:
$S0 = typeof $P0
L2:
say $S0
.end
Expand All @@ -2506,7 +2511,7 @@ pir_output_is( <<'CODE', <<'OUTPUT', "Handling :flat of empty arguments" );
.end
CODE
ResizablePMCArray
Undef
PMCNULL
OUTPUT

pir_output_is( <<'CODE', <<'OUTPUT', "Tailcall from vtable" );
Expand Down

0 comments on commit 6fe1c97

Please sign in to comment.