diff --git a/src/Changes b/src/Changes index ef7a906913..0a9f35a5c1 100644 --- a/src/Changes +++ b/src/Changes @@ -1,5 +1,22 @@ Version 7.1 +10/8/26 [GH #178] +Constexpr call depth limit and calls in arguments + +The constexpr interpreter limits the nesting of calls to the value of the +--max_depth_constexpr_call option by charging each call a large up-front cost +that is refunded when the call completes. That cost was charged before the +arguments of the call were evaluated, so a call appearing in an argument was +treated as nested in the call to which the argument is passed, even though +the latter has not yet been entered. For example, with --c++17 +and --max_depth_constexpr_call=300: + + constexpr int add(int a, int b) { return a + b; } + constexpr int f(int n) { return n == 0 ? 0 : add(1, f(n - 1)); } + static_assert(f(256) == 256, ""); // Previously not a constant. + +Constructor calls had the same problem. This is now fixed. + 10/7/26 [GH #176] C-generating back end: always_inline attribute and inlined setjmp calls diff --git a/src/interpret.c b/src/interpret.c index b9b7e62341..1ebcfc1719 100644 --- a/src/interpret.c +++ b/src/interpret.c @@ -22952,7 +22952,6 @@ callee; the iwp_1st_resume visit completes the call in the latter case. a_boolean eval_right_to_left = call_node->variant.operation.eval_right_to_left; work_item_result_cap(item, &result_cap); - up_front_cost = charge_call_cost(ips); /* Set up arguments, starting with "this" if applicable. */ /* This process must happen in two phases. First, the arguments must be allocated and evaluated. Only then can we map parameter variables onto @@ -23165,6 +23164,7 @@ callee; the iwp_1st_resume visit completes the call in the latter case. should perform some more work (like mapping parameters) in case the intrinsic handling falls back to the provided definition (as, e.g., std::construct_at does). */ + up_front_cost = charge_call_cost(ips); push_new_call_frame(ips, callee, &call_node->position, result_cap); result = do_constexpr_intrinsic_call( ips, callee, call_node, (a_byte**)arg_ptrs, @@ -23254,6 +23254,11 @@ callee; the iwp_1st_resume visit completes the call in the latter case. p_arg_ptr += 1; arg_size += 1; } /* for */ + /* Charge for the call only now that the arguments have been evaluated. + A call nested in an argument (e.g., the "f(n-1)" in "g(f(n-1))") is + not nested in this call, and charging earlier would make it count + twice toward the call depth limit. */ + up_front_cost = charge_call_cost(ips); /* Set up the call frame. */ push_new_call_frame(ips, callee, &call_node->position, result_cap); /* Record what finish_call_work needs to undo all of the above. */ @@ -23499,7 +23504,6 @@ after the constructor body has been interpreted. class_type, ips); goto done; } /* if */ - up_front_cost = charge_call_cost(ips); /* Set up arguments, starting with "this" if applicable. */ /* This process must happen in two phases. First, the arguments must be allocated and evaluated. Only then can we map parameter variables onto @@ -23656,6 +23660,9 @@ after the constructor body has been interpreted. memzero(result_storage+sizeof(void*), size_t_arg(n_class_bytes-sizeof(void*))); } /* if */ + /* Charge for the call only now that the arguments have been evaluated + (see process_call_work). */ + up_front_cost = charge_call_cost(ips); /* Set up the call frame. */ push_new_call_frame(ips, callee, pos, cap); /* Mark all the empty base class subobjects as initialized. */ diff --git a/tests/tests/regressions/.gh-178-fn.rto/default.1.1.txt b/tests/tests/regressions/.gh-178-fn.rto/default.1.1.txt new file mode 100644 index 0000000000..ceb888af1b --- /dev/null +++ b/tests/tests/regressions/.gh-178-fn.rto/default.1.1.txt @@ -0,0 +1,9 @@ +fe_only -DTEST_NUMBER=1 --c++17 --max_depth_constexpr_call=300 Test_name.c +"Test_name.c", line 11: error: expression must have a constant value + static_assert(f(300) == 300, ""); + ^ +"Test_name.c", line 11: note: expression not folded to a constant due to excessive constexpr function call complexity + static_assert(f(300) == 300, ""); + ^ + +1 error detected in the compilation of "Test_name.c". diff --git a/tests/tests/regressions/.gh-178.rto/default.1.1.txt b/tests/tests/regressions/.gh-178.rto/default.1.1.txt new file mode 100644 index 0000000000..279211d2c5 --- /dev/null +++ b/tests/tests/regressions/.gh-178.rto/default.1.1.txt @@ -0,0 +1 @@ +fe_only -DTEST_NUMBER=1 --c++17 --max_depth_constexpr_call=300 Test_name.c diff --git a/tests/tests/regressions/gh-178-fn.sft.cpp b/tests/tests/regressions/gh-178-fn.sft.cpp new file mode 100644 index 0000000000..0194771dbd --- /dev/null +++ b/tests/tests/regressions/gh-178-fn.sft.cpp @@ -0,0 +1,11 @@ +//type:fn +//options_all:--c++17 --max_depth_constexpr_call=300 + +// Companion to gh-178.sft.cpp: a call in an argument no longer counts as +// nested in the call it is passed to, but the depth limit is still enforced. + +constexpr int add(int a, int b) { return a + b; } +constexpr int f(int n) { return n == 0 ? 0 : add(1, f(n - 1)); } + +// 301 nested calls of f: one call too many, still beyond the limit. +static_assert(f(300) == 300, ""); diff --git a/tests/tests/regressions/gh-178.sft.cpp b/tests/tests/regressions/gh-178.sft.cpp new file mode 100644 index 0000000000..50e525be69 --- /dev/null +++ b/tests/tests/regressions/gh-178.sft.cpp @@ -0,0 +1,20 @@ +//type:fp +//options_all:--c++17 --max_depth_constexpr_call=300 + +// A constexpr call that appears in an argument of another constexpr call is +// evaluated before that other call is entered, so it is not nested in it. +// Previously, the interpreter charged the outer call against the call depth +// limit before evaluating its arguments, so each level of recursion below +// counted twice and only about half the requested depth was available. + +constexpr int add(int a, int b) { return a + b; } +constexpr int f(int n) { return n == 0 ? 0 : add(1, f(n - 1)); } +static_assert(f(256) == 256, ""); // Previously not a constant. +static_assert(f(299) == 299, ""); // 300 nested calls of f: at the limit. + +struct S { + int v; + constexpr S(int x) : v(x) {} +}; +constexpr int g(int n) { return n == 0 ? 0 : S(1 + g(n - 1)).v; } +static_assert(g(299) == 299, ""); // Previously not a constant.