Skip to content

Commit 041a403

Browse files
python-processing-unitpython-processing-unit
committed
gh-133: Fix FOR counter handling.
1 parent f22ceb0 commit 041a403

6 files changed

Lines changed: 111 additions & 17 deletions

File tree

‎src/env.c‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,53 @@ bool env_set_alias_cross(Env* env, const char* name, Env* target_env, const char
452452
return true;
453453
}
454454

455+
bool env_restore_local_direct(Env* env, const char* name, Value value, DeclType type, bool initialized) {
456+
if (!env || !name) return false;
457+
458+
EnvEntry* entry = env_find_local(env, name);
459+
if (!entry) {
460+
if (!env_define_direct(env, name, type)) return false;
461+
entry = env_find_local(env, name);
462+
if (!entry) return false;
463+
}
464+
465+
/* Respect frozen/permafrozen state */
466+
if (entry->frozen || entry->permafrozen) return false;
467+
468+
/* Overwrite declared type */
469+
entry->decl_type = type;
470+
471+
/* Remove any aliasing */
472+
if (entry->alias_target) {
473+
free(entry->alias_target);
474+
entry->alias_target = NULL;
475+
entry->alias_target_env = NULL;
476+
}
477+
478+
/* Replace stored value if present */
479+
if (entry->initialized) {
480+
value_free(entry->value);
481+
entry->initialized = false;
482+
entry->value = value_null();
483+
}
484+
485+
if (initialized) {
486+
entry->value = value_copy(value);
487+
entry->initialized = true;
488+
} else {
489+
entry->initialized = false;
490+
entry->value = value_null();
491+
}
492+
493+
return true;
494+
}
495+
496+
bool env_restore_local(Env* env, const char* name, Value value, DeclType type, bool initialized) {
497+
if (ns_buffer_active() && !ns_buffer_is_prepare_thread())
498+
return ns_buffer_restore_local(env, name, value, type, initialized);
499+
return env_restore_local_direct(env, name, value, type, initialized);
500+
}
501+
455502
int env_freeze_direct(Env* env, const char* name) {
456503
EnvEntry* entry = env_get_entry_raw(env, name);
457504
if (!entry) return -1;

‎src/env.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,11 @@ int env_freeze_direct(Env* env, const char* name);
8686
int env_thaw_direct(Env* env, const char* name);
8787
int env_permafreeze_direct(Env* env, const char* name);
8888

89+
// Restore or overwrite a local entry's declared type and value. This will
90+
// update the local entry (creating it if absent) to have `type` and, if
91+
// `initialized` is true, set its value to a copy of `value` and mark it
92+
// initialized; otherwise leave it uninitialized. Returns true on success.
93+
bool env_restore_local(Env* env, const char* name, Value value, DeclType type, bool initialized);
94+
bool env_restore_local_direct(Env* env, const char* name, Value value, DeclType type, bool initialized);
95+
8996
#endif // ENV_H

‎src/interpreter.c‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3232,6 +3232,30 @@ static ExecResult exec_stmt(Interpreter* interp, Stmt* stmt, Env* env, LabelMap*
32323232
bool prev_initialized = false;
32333233
env_get(env, stmt->as.for_stmt.counter, &prev_val, &prev_type, &prev_initialized);
32343234

3235+
/* Track whether the counter name exists in any parent env.
3236+
Loop-local binding must shadow the global symbol and restore it
3237+
after the loop, regardless of where it was originally defined.
3238+
We cannot rely on `env_get` alone because it doesn't distinguish
3239+
between current-env and parent-env bindings. */
3240+
bool had_global = false;
3241+
Value global_prev_val = value_null();
3242+
DeclType global_prev_type = TYPE_UNKNOWN;
3243+
bool global_prev_initialized = false;
3244+
for (Env* cursor = env; cursor && cursor->parent; cursor = cursor->parent) {
3245+
Env* parent = cursor->parent;
3246+
if (!parent) break;
3247+
Value tmp_val = value_null();
3248+
DeclType tmp_type = TYPE_UNKNOWN;
3249+
bool tmp_initialized = false;
3250+
if (env_get(parent, stmt->as.for_stmt.counter, &tmp_val, &tmp_type, &tmp_initialized)) {
3251+
had_global = true;
3252+
global_prev_val = tmp_val;
3253+
global_prev_type = tmp_type;
3254+
global_prev_initialized = tmp_initialized;
3255+
break;
3256+
}
3257+
}
3258+
32353259
/* Detect whether a local binding already exists (trial define) */
32363260
bool local_existed = true;
32373261
if (env_define(env, stmt->as.for_stmt.counter, TYPE_INT)) {
@@ -3275,8 +3299,7 @@ static ExecResult exec_stmt(Interpreter* interp, Stmt* stmt, Env* env, LabelMap*
32753299
env_delete(env, stmt->as.for_stmt.counter);
32763300
env_delete(env, temp_name);
32773301
if (local_existed) {
3278-
env_define(env, stmt->as.for_stmt.counter, prev_type);
3279-
if (prev_initialized) env_assign(env, stmt->as.for_stmt.counter, prev_val, prev_type, false);
3302+
env_restore_local(env, stmt->as.for_stmt.counter, prev_val, prev_type, prev_initialized);
32803303
}
32813304
value_free(prev_val);
32823305
interp->loop_depth--;
@@ -3291,8 +3314,7 @@ static ExecResult exec_stmt(Interpreter* interp, Stmt* stmt, Env* env, LabelMap*
32913314
env_delete(env, stmt->as.for_stmt.counter);
32923315
env_delete(env, temp_name);
32933316
if (local_existed) {
3294-
env_define(env, stmt->as.for_stmt.counter, prev_type);
3295-
if (prev_initialized) env_assign(env, stmt->as.for_stmt.counter, prev_val, prev_type, false);
3317+
env_restore_local(env, stmt->as.for_stmt.counter, prev_val, prev_type, prev_initialized);
32963318
}
32973319
value_free(prev_val);
32983320
interp->loop_depth--;
@@ -3306,8 +3328,7 @@ static ExecResult exec_stmt(Interpreter* interp, Stmt* stmt, Env* env, LabelMap*
33063328
env_delete(env, stmt->as.for_stmt.counter);
33073329
env_delete(env, temp_name);
33083330
if (local_existed) {
3309-
env_define(env, stmt->as.for_stmt.counter, prev_type);
3310-
if (prev_initialized) env_assign(env, stmt->as.for_stmt.counter, prev_val, prev_type, false);
3331+
env_restore_local(env, stmt->as.for_stmt.counter, prev_val, prev_type, prev_initialized);
33113332
}
33123333
value_free(prev_val);
33133334
interp->loop_depth--;
@@ -3321,8 +3342,7 @@ static ExecResult exec_stmt(Interpreter* interp, Stmt* stmt, Env* env, LabelMap*
33213342
env_delete(env, stmt->as.for_stmt.counter);
33223343
env_delete(env, temp_name);
33233344
if (local_existed) {
3324-
env_define(env, stmt->as.for_stmt.counter, prev_type);
3325-
if (prev_initialized) env_assign(env, stmt->as.for_stmt.counter, prev_val, prev_type, false);
3345+
env_restore_local(env, stmt->as.for_stmt.counter, prev_val, prev_type, prev_initialized);
33263346
}
33273347
value_free(prev_val);
33283348
interp->loop_depth--;
@@ -3338,8 +3358,14 @@ static ExecResult exec_stmt(Interpreter* interp, Stmt* stmt, Env* env, LabelMap*
33383358
env_delete(env, stmt->as.for_stmt.counter);
33393359
env_delete(env, temp_name);
33403360
if (local_existed) {
3341-
env_define(env, stmt->as.for_stmt.counter, prev_type);
3342-
if (prev_initialized) env_assign(env, stmt->as.for_stmt.counter, prev_val, prev_type, false);
3361+
env_restore_local(env, stmt->as.for_stmt.counter, prev_val, prev_type, prev_initialized);
3362+
}
3363+
3364+
/* Restore global binding if the aliasing process shadowed a
3365+
non-local (global/parent) symbol with a non-INT type. */
3366+
if (had_global) {
3367+
env_assign(env, stmt->as.for_stmt.counter, global_prev_val, global_prev_type, false);
3368+
value_free(global_prev_val);
33433369
}
33443370
value_free(prev_val);
33453371

‎src/ns_buffer.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ static void execute_op(NsOp* op) {
118118
op->target_name, op->decl_type,
119119
op->declare_if_missing);
120120
break;
121+
case NS_OP_RESTORE:
122+
op->result_ok = env_restore_local_direct(op->env, op->name, op->value, op->decl_type, op->declare_if_missing);
123+
break;
121124
case NS_OP_FREEZE:
122125
op->result_int = env_freeze_direct(op->env, op->name);
123126
break;
@@ -461,6 +464,19 @@ bool ns_buffer_set_alias(struct Env* env, const char* name,
461464
return r;
462465
}
463466

467+
bool ns_buffer_restore_local(struct Env* env, const char* name, Value value, DeclType type, bool initialized) {
468+
NsOp* op = make_op(NS_OP_RESTORE, env, name);
469+
op->value = value_copy(value);
470+
op->decl_type = type;
471+
op->declare_if_missing = initialized;
472+
enqueue_op(op);
473+
wait_op(op);
474+
bool r = op->result_ok;
475+
value_free(op->value);
476+
free_op(op);
477+
return r;
478+
}
479+
464480
int ns_buffer_freeze(struct Env* env, const char* name) {
465481
NsOp* op = make_op(NS_OP_FREEZE, env, name);
466482
enqueue_op(op);

‎src/ns_buffer.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ typedef enum {
1414
NS_OP_INDEX_ASSIGN,
1515
NS_OP_DELETE,
1616
NS_OP_ALIAS,
17+
NS_OP_RESTORE,
1718
NS_OP_FREEZE,
1819
NS_OP_THAW,
1920
NS_OP_PERMAFREEZE
@@ -124,6 +125,7 @@ bool ns_buffer_delete(struct Env* env, const char* name);
124125
bool ns_buffer_set_alias(struct Env* env, const char* name,
125126
const char* target_name, DeclType type,
126127
bool declare_if_missing);
128+
bool ns_buffer_restore_local(struct Env* env, const char* name, Value value, DeclType type, bool initialized);
127129
int ns_buffer_freeze(struct Env* env, const char* name);
128130
int ns_buffer_thaw(struct Env* env, const char* name);
129131
int ns_buffer_permafreeze(struct Env* env, const char* name);

‎tests/cases/passing/for-zero-and-restore-counter.pre‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
BOOL: i = TRUE
2-
INT: runs = 0d0
3-
FOR(i, 0d0){
4-
ADD(@runs, 0d1)
5-
}
6-
ASSERT(EQ(runs, 0d0))
7-
ASSERT(i)
1+
STR: i = "foo"
2+
FOR(i, 0d10){}
3+
ASSERT(EQ(i, "foo"))

0 commit comments

Comments
 (0)