Skip to content

Commit 4783583

Browse files
borisbatclaude
andauthored
interp: block return must not ride the invokeEx code0 fast path (#3572)
* interp: block return must not ride the invokeEx code0 fast path Context::invokeEx hands enumerators (for_each_function et al.) the closure body's single statement bare when code0 is set — skipping the SimNode_ClosureBlock epilogue that clears stopForReturn. A firing bare `return` inside such a block then killed the remaining iterations AND leaked the stop flag into the calling function, silently skipping its tail. Longest-lived visible casualty: rst.das documents() dropped every Uncategorized function from the generated docs, and gate 3 passed precisely because the loop was dead. JIT and AOT were always correct. Fix: C0 additionally requires !expr->hasReturn, so return-bearing single-statement closures take the full ClosureBlock path. Failing test first: tests/ast/test_closure_block_return.das (red pre-fix, green now). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q8bZnWkVWeXy8jfrxSojso * docs: group the functions the healed Uncategorized loop surfaced The invokeEx block-return fix brings rst.das documents()'s Uncategorized loop back to life, surfacing functions that had been silently dropped from the generated docs. Groups + handmade entries for the new ones: fio set_env_variable + long_fread/long_fwrite, logger_init_tee/ logger_set_tee, and the per-thread jobque dials (set/get_jobque_thread_ team_mode, set_current_thread_priority — hidden with the tuning-knob family). Existing jobque handmade descriptions are untouched (an earlier revision of this branch accidentally rewrote them; restored verbatim — Copilot review caught the divergences). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q8bZnWkVWeXy8jfrxSojso --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 8433389 commit 4783583

7 files changed

Lines changed: 76 additions & 6 deletions

doc/reflections/das2rst.das

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,12 @@ def document_module_fio(_root : string) {
214214
var mod = [get_module("fio_core"), find_module("fio")]
215215
var groups <- array<DocGroup>(
216216
hide_group(group_by_regex("Internal builtin functions", mod, %regex~builtin%%)),
217-
group_by_regex("File manipulation", mod, %regex~(f|stat$|getchar|remove|rename|copy_file|file_size|equivalent|is_symlink|set_mtime)%%),
217+
group_by_regex("File manipulation", mod, %regex~(f|long_f|stat$|getchar|remove|rename|copy_file|file_size|equivalent|is_symlink|set_mtime)%%),
218218
group_by_regex("Path manipulation", mod, %regex~(base_name|dir_name|get_full_file_name|extension|stem|replace_extension|path_join|normalize|to_generic_path|is_absolute|relative|relative_result|parent)$%%),
219219
group_by_regex("Directory manipulation", mod, %regex~(dir|dir_rec|mkdir|mkdir_rec|mkdir_result|rmdir|rmdir_rec|rmdir_rec_result|rmdir_result|chdir|getcwd)$%%),
220220
group_by_regex("Glob and pattern matching", mod, %regex~(match_glob|glob|glob_filtered|is_glob_pattern|expand_glob|parse_file_list)$%%),
221221
group_by_regex("Filesystem queries", mod, %regex~(temp_directory|temp_directory_result|create_temp_file|create_temp_file_result|create_temp_directory|create_temp_directory_result|disk_space)$%%),
222-
group_by_regex("OS specific routines", mod, %regex~(sleep|exit|system|popen|popen_binary|popen_timeout|spawn_argv|popen_argv|popen_argv_pipe|popen_timed_out|run_and_capture|get_env_variable|sanitize_command_line|has_env_variable)$%%),
222+
group_by_regex("OS specific routines", mod, %regex~(sleep|exit|system|popen|popen_binary|popen_timeout|spawn_argv|popen_argv|popen_argv_pipe|popen_timed_out|run_and_capture|get_env_variable|set_env_variable|sanitize_command_line|has_env_variable)$%%),
223223
group_by_regex("Dynamic modules", mod, %regex~(register_dynamic_module|register_native_path)$%%)
224224
)
225225
documents("File input output library", mod, "fio.rst", groups)
@@ -598,7 +598,7 @@ def document_module_jobque(_root : string) {
598598
group_by_regex("Channel, JobStatus, Lockbox, Stream", mod, %regex~(append|notify|join|channel_create|channel_remove|stream_create|stream_remove|add_ref|release|notify_and_release|lock_box_create|lock_box_remove|job_status_create|job_status_remove)$%%),
599599
group_by_regex("Queries", mod, %regex~(get_total_hw_jobs|get_total_hw_threads|get_total_hw_cores|is_job_que_shutting_down|is_job_que_available|count_jobque_leaks)$%%),
600600
group_by_regex("Event tracing", mod, %regex~(jobque_trace_start|jobque_trace_stop|jobque_trace_save|jobque_trace_tag|jobque_trace_category|jobque_trace_marker_name|jobque_trace_marker)$%%),
601-
group_by_regex("Internal invocations", mod, %regex~(new_job_invoke|new_thread_invoke|new_debugger_thread|set_jobque_fork_pool|set_jobque_fork_skip_heap_reset|set_jobque_worker_spin|set_jobque_batch_dispatch|set_jobque_join_spin|set_jobque_team_mode|get_jobque_team_mode|set_jobque_threads_cap|set_jobque_affinity|get_jobque_affinity|set_jobque_worker_limit|get_jobque_worker_limit|set_jobque_team_rank_gate|get_jobque_team_rank_gate|set_jobque_team_prof|reset_jobque_team_prof|get_jobque_team_prof|get_jobque_team_prof_counts|get_jobque_team_prof_react|team_parallel_for_invoke|team_parallel_for_indexed_invoke|team_parallel_stages_invoke|flush_jobque_batch|jobque_try_run_one)$%%),
601+
group_by_regex("Internal invocations", mod, %regex~(new_job_invoke|new_thread_invoke|new_debugger_thread|set_jobque_fork_pool|set_jobque_fork_skip_heap_reset|set_jobque_worker_spin|set_jobque_batch_dispatch|set_jobque_join_spin|set_jobque_team_mode|get_jobque_team_mode|set_jobque_thread_team_mode|get_jobque_thread_team_mode|set_current_thread_priority|set_jobque_threads_cap|set_jobque_affinity|get_jobque_affinity|set_jobque_worker_limit|get_jobque_worker_limit|set_jobque_team_rank_gate|get_jobque_team_rank_gate|set_jobque_team_prof|reset_jobque_team_prof|get_jobque_team_prof|get_jobque_team_prof_counts|get_jobque_team_prof_react|team_parallel_for_invoke|team_parallel_for_indexed_invoke|team_parallel_stages_invoke|flush_jobque_batch|jobque_try_run_one)$%%),
602602
group_by_regex("Construction", mod, %regex~(with_channel|with_stream|with_job_status|with_job_que|with_lock_box|create_job_que|destroy_job_que)$%%),
603603
group_by_regex("Atomic", mod, %regex~(atomic32_create|atomic32_remove|with_atomic32|atomic64_create|atomic64_remove|with_atomic64|get|set|inc|dec)$%%)
604604
)
@@ -1316,9 +1316,9 @@ def document_module_toml(_root : string) {
13161316
def document_module_logger(_root : string) {
13171317
var mod = find_module("logger")
13181318
var groups <- array<DocGroup>(
1319-
group_by_regex("Setup", mod, %regex~.*(logger_init|logger_set_name|logger_set_path|logger_get_path|logger_get_name)$%%),
1319+
group_by_regex("Setup", mod, %regex~.*(logger_init|logger_init_tee|logger_set_name|logger_set_path|logger_get_path|logger_get_name)$%%),
13201320
group_by_regex("Level filters", mod, %regex~.*(logger_set_min_level|logger_get_min_level|logger_set_category_level|logger_clear_category_levels)$%%),
1321-
group_by_regex("Output control", mod, %regex~.*(logger_set_stderr_fallback|logger_flush|logger_close)$%%),
1321+
group_by_regex("Output control", mod, %regex~.*(logger_set_stderr_fallback|logger_set_tee|logger_flush|logger_close)$%%),
13221322
group_by_regex("Log calls", mod, %regex~.*(logger_log|logger_trace|logger_debug|logger_info|logger_warning|logger_error|logger_critical)$%%),
13231323
group_by_regex("Stdout hook", mod, %regex~.*(logger_install_hook)$%%)
13241324
)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Sets an environment variable in the current process (the `setenv`/`_putenv` family). The change is visible to subsequent `get_env_variable` calls and inherited by child processes spawned afterwards (`popen`, `spawn_argv`); it does not affect the parent shell.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Reads the calling OS thread's team-mode override (see `set_jobque_thread_team_mode`).
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Sets the OS priority of the calling thread on the jobque `JobPriority` scale, -2 (Minimum) .. 2 (Maximum), clamped. The dispatch caller is load-bearing in team mode — only it publishes chains, so a descheduled caller idles every lane; queue creation already runs it at High (+1), and this exposes the remaining notch (and the rest of the scale) to scripts.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Per-caller team-mode override: with team mode off for this OS thread, `team_parallel_*` calls run inline on the caller while other threads keep publishing to the shared worker team. New OS threads default to enabled.

src/ast/ast_simulate.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2221,7 +2221,10 @@ namespace das
22212221
SimNode_Block * block;
22222222
if ( expr->isClosure ) {
22232223
bool needResult = expr->type!=nullptr && expr->type->baseType!=Type::tVoid;
2224-
bool C0 = !needResult && simlist.size()==1 && expr->finalList.size()==0;
2224+
// the invokeEx code0 fast path evals list[0] bare, skipping the ClosureBlock
2225+
// epilogue that clears stopForReturn — a block containing a return must take
2226+
// the full path or the flag leaks into the invoking enumeration and its caller
2227+
bool C0 = !needResult && simlist.size()==1 && expr->finalList.size()==0 && !expr->hasReturn;
22252228
#if DAS_DEBUGGER
22262229
if ( context.debugger ) {
22272230
block = context.code->makeNode<SimNodeDebug_ClosureBlock>(at, needResult, C0, expr->annotationData);
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
options gen2
2+
options indenting = 4
3+
options rtti
4+
5+
require dastest/testing_boost public
6+
require daslib/ast
7+
8+
//! A bare `return` inside a block exits THE BLOCK only. The interpreter's invokeEx code0 fast
9+
//! path handed enumerators (for_each_function et al.) the closure body's single statement WITHOUT
10+
//! the SimNode_ClosureBlock epilogue that clears stopForReturn — so the first firing return
11+
//! killed the remaining iterations AND leaked the stop flag into the calling function, silently
12+
//! skipping its tail (rst.das documents() lost every Uncategorized function this way). The
13+
//! single-statement and multi-statement closures below are semantically identical; they must
14+
//! count the same. JIT and AOT were always correct — this is an interpreter-only regression rail.
15+
16+
// single-statement closure body (one IfThenElse) — rides the code0 fast path
17+
def private count_single(mod : rtti::Module?) : int {
18+
var n = 0
19+
for_each_function(mod, "") $(func) {
20+
if (func.flags.generated) {
21+
return
22+
} else {
23+
n++
24+
}
25+
}
26+
return n
27+
}
28+
29+
// multi-statement closure body — never rode the fast path, always correct
30+
def private count_multi(mod : rtti::Module?; var fired : int&) : int {
31+
var n = 0
32+
for_each_function(mod, "") $(func) {
33+
if (func.flags.generated) {
34+
fired++
35+
return
36+
}
37+
n++
38+
}
39+
return n
40+
}
41+
42+
// tail sentinel: a leaked stop flag would skip everything after the enumeration
43+
def private tail_runs(mod : rtti::Module?) : bool {
44+
for_each_function(mod, "") $(func) {
45+
if (func.flags.generated) {
46+
return
47+
}
48+
}
49+
return true
50+
}
51+
52+
[test]
53+
def test_closure_block_return(t : T?) {
54+
ast_gc_guard() {
55+
var mod = get_module("$")
56+
var fired = 0
57+
let multi = count_multi(mod, fired)
58+
t |> success(fired > 0, "the early return must actually fire (module $ has generated fns)")
59+
t |> success(multi > 0, "enumeration continues past a block return")
60+
t |> equal(count_single(mod), multi)
61+
t |> success(tail_runs(mod), "block return must not leak past the enumeration")
62+
}
63+
}

0 commit comments

Comments
 (0)