diff --git a/include/numsim_codegen/code_emit/codegen_context.h b/include/numsim_codegen/code_emit/codegen_context.h index 5ae8c47..2a648fb 100644 --- a/include/numsim_codegen/code_emit/codegen_context.h +++ b/include/numsim_codegen/code_emit/codegen_context.h @@ -10,6 +10,7 @@ #include #include #include +#include #include namespace numsim::codegen { @@ -46,6 +47,14 @@ class CodeGenContext { m_named_symbols[&expr.get()] = name; } + // Reserve an identifier so fresh_name() will never produce it (issue #8): + // a user symbol named `t0` would otherwise be REDECLARED by the first + // generated temporary (`auto t0 = ...;`). Callers seed every declared + // name (symbols, outputs + `_out`) before emission. Reserving a + // name never changes the numbering of non-colliding temporaries — the + // counter only skips over reserved candidates. + void reserve_name(std::string name) { m_reserved.insert(std::move(name)); } + // CSE table operations. [[nodiscard]] auto find(void const *ptr) const -> std::string const * { if (auto it = m_cse_table.find(ptr); it != m_cse_table.end()) { @@ -111,28 +120,40 @@ class CodeGenContext { // Clear statements and CSE table for a new emission pass. The temporary // counter is intentionally NOT reset — that way, if two rendered outputs // ever end up in the same scope (e.g. concatenated into one function), - // their names cannot collide. Call full_reset() if you genuinely want - // the counter zeroed. + // their names cannot collide. Reserved names are likewise preserved: + // they describe the DECLARED identifiers of the surrounding function + // (like the symbol registrations), which stay in scope across the + // per-block reset() calls within one emission. Call full_reset() if you + // genuinely want the counter zeroed and the reservations dropped. void reset() { m_statements.clear(); m_cse_table.clear(); m_shared_table.clear(); - // m_counter and m_named_symbols deliberately preserved. + // m_counter, m_named_symbols and m_reserved deliberately preserved. } void full_reset() { reset(); m_counter = 0; m_named_symbols.clear(); + m_reserved.clear(); } private: - auto fresh_name() -> std::string { return "t" + std::to_string(m_counter++); } + auto fresh_name() -> std::string { + for (;;) { + auto name = "t" + std::to_string(m_counter++); + if (!m_reserved.contains(name)) { + return name; + } + } + } std::vector m_statements; std::unordered_map m_cse_table; std::unordered_map m_shared_table; std::unordered_map m_named_symbols; + std::unordered_set m_reserved; int m_counter = 0; }; diff --git a/include/numsim_codegen/passes/internal/pass_bodies.h b/include/numsim_codegen/passes/internal/pass_bodies.h index 306d4a1..1c41ff9 100644 --- a/include/numsim_codegen/passes/internal/pass_bodies.h +++ b/include/numsim_codegen/passes/internal/pass_bodies.h @@ -520,6 +520,14 @@ inline void CodeEmitPass::run(PassContext &pctx) { for (auto const &[name, expr] : model.tensor_symbol_map()) { ctx.register_symbol_tensor(expr, name); } + // Issue #8: reserve every declared identifier (symbols incl. `_old` + // entries, outputs + `_out`, tangents) so a user symbol named like + // a generated temporary (`t0`, `t1`, …) is never redeclared by the CSE. + // Reservations survive the per-block ctx.reset() calls below, exactly + // like the symbol registrations they guard. + for (auto const &name : detail::emission_reserved_names(model)) { + ctx.reserve_name(name); + } // Phase 3a-2: render each Newton segment's residual + Jacobian with // LOOP-LOCAL CSE. `ctx.reset()` clears statements + the CSE table but diff --git a/include/numsim_codegen/recipe.h b/include/numsim_codegen/recipe.h index 025b5a8..a326535 100644 --- a/include/numsim_codegen/recipe.h +++ b/include/numsim_codegen/recipe.h @@ -1529,6 +1529,30 @@ inline auto tensor_arg_count(RecipeView model) -> int { return n; } +// Issue #8: every identifier the generated compute function declares or +// references verbatim — symbol names (inputs, parameters, state variables +// including their `_old` entries, dt), output names plus their `_out` +// out-params, and tangent names (which materialise as tangent out-params +// whether or not AlgorithmicTangentPass has run yet). Generated identifiers +// must skip these: a scalar input named `t0` would otherwise be REDECLARED +// by the first CSE temporary, and a tensor input named `T0` would SHADOW +// its own template parameter. +inline auto emission_reserved_names(RecipeView model) -> std::set { + std::set names; + for (auto const &s : model.symbols()) { + names.insert(s.name); + } + for (auto const &o : model.outputs()) { + names.insert(o.name); + names.insert(o.name + "_out"); + } + for (auto const &t : model.raw_model().tangents()) { + names.insert(t.name); + names.insert(t.name + "_out"); + } + return names; +} + } // namespace detail // Phase 2.6 (issue #77): the single source of truth for the generated @@ -1695,16 +1719,33 @@ inline auto render_compute_function( os << "// Auto-generated by numsim-codegen. Do not edit.\n"; os << "// Model: " << model.name() << "\n\n"; + // Every identifier the recipe declares (issue #8) — template parameter + // names below and the coupled-Newton solve-locals further down must not + // collide with any of them. + auto const declared_ids = detail::emission_reserved_names(model); + // Template parameter list: one T per tensor argument so the caller // can pass any tmech::tensor_base subclass (tensor, adaptor, expression - // template) without forcing a materialised copy. + // template) without forcing a materialised copy. Candidates colliding + // with a declared identifier are skipped (issue #8): `template (T0 const &T0)` would shadow the template parameter with the + // argument. Skipping only on collision keeps the T-numbering of every + // collision-free recipe byte-identical. int const n_tmpl = detail::tensor_arg_count(model); + std::vector tmpl_names; + tmpl_names.reserve(static_cast(n_tmpl)); + for (int i = 0; static_cast(tmpl_names.size()) < n_tmpl; ++i) { + auto candidate = "T" + std::to_string(i); + if (!declared_ids.contains(candidate)) { + tmpl_names.push_back(std::move(candidate)); + } + } if (n_tmpl > 0) { os << "template <"; for (int i = 0; i < n_tmpl; ++i) { if (i > 0) os << ", "; - os << "typename T" << i; + os << "typename " << tmpl_names[static_cast(i)]; } os << ">\n"; } @@ -1712,7 +1753,7 @@ inline auto render_compute_function( // Signature built from the canonical argument list (issue #77) — the // exact order backends must reproduce in their call-sites. - int tmpl_counter = 0; + std::size_t tmpl_counter = 0; bool first = true; for (auto const &a : canonical_arguments(model)) { if (!first) { @@ -1724,13 +1765,13 @@ inline auto render_compute_function( // so adding an ArgSpec::Role is a -Wswitch compile warning here. switch (a.role) { case ArgSpec::Role::TensorInput: - os << "T" << tmpl_counter++ << " const &" << a.name; + os << tmpl_names[tmpl_counter++] << " const &" << a.name; break; case ArgSpec::Role::TensorOutput: case ArgSpec::Role::TensorTangentOutput: // Identical in the target-agnostic signature — a rank-4 out-param. The // tangent-vs-ordinary distinction is a backend call-site concern (Phase 5). - os << "T" << tmpl_counter++ << " &" << a.name << "_out"; + os << tmpl_names[tmpl_counter++] << " &" << a.name << "_out"; break; case ArgSpec::Role::ScalarOutput: case ArgSpec::Role::NewtonStateOut: @@ -1782,15 +1823,9 @@ inline auto render_compute_function( if (!newton_systems.empty()) { auto suffixes = la.local_suffixes(); suffixes.emplace_back("iter"); // render's loop counter - std::set reserved_ids; - for (auto const &[nm, _] : model.scalar_symbol_map()) - reserved_ids.insert(nm); - for (auto const &[nm, _] : model.tensor_symbol_map()) - reserved_ids.insert(nm); - for (auto const &o : model.outputs()) { - reserved_ids.insert(o.name); - reserved_ids.insert(o.name + "_out"); - } + // Same declared-identifier set as the template-parameter skip above + // (issue #8 unified the two enumerations into emission_reserved_names). + auto const &reserved_ids = declared_ids; for (auto const &sys : newton_systems) { std::string p = sys.unknowns[0]; // seed; mangled below until collision-free auto collides = [&](std::string const &pre) { diff --git a/src/targets/numsim_material.cpp b/src/targets/numsim_material.cpp index 640a103..7f85438 100644 --- a/src/targets/numsim_material.cpp +++ b/src/targets/numsim_material.cpp @@ -352,8 +352,15 @@ std::vector emit_residual_material(ConstitutiveModel const &model) // Register scalar symbols (state→local bare name, params→m_) and tensor // symbols (input→local bare name) into an emit context. Shared by the residual - // /jacobian render and reused (fresh contexts) per output. + // /jacobian render and reused (fresh contexts) per output. Declared names are + // reserved so a symbol named like a CSE temporary (`t0`, …) is never + // redeclared by the generated locals (issue #8). + auto const reserved_names = + detail::emission_reserved_names(RecipeView{model}); auto register_all = [&](CodeGenContext &c) { + for (auto const &n : reserved_names) { + c.reserve_name(n); + } for (auto const &[name, expr] : model.scalar_symbol_map()) { if (param_names.contains(name)) { c.register_symbol_scalar(expr, "m_" + name); @@ -1004,8 +1011,15 @@ auto NumSimMaterialTarget::emit(ConstitutiveModel const &model) const // Emit-name mapping: the state current → a local (bare `cur_name`, bound from // m_.get()); parameters → their member `m_`; the rate-leaf guard // above guarantees nothing else can appear. + // Declared names are reserved in every emit context here so a symbol named + // like a CSE temporary (`t0`, …) is never redeclared (issue #8). + auto const reserved_names = + detail::emission_reserved_names(RecipeView{model}); CodeGenContext ctx; CodeEmitPipeline pipeline(ctx); + for (auto const &n : reserved_names) { + ctx.reserve_name(n); + } for (auto const &[name, expr] : model.scalar_symbol_map()) { if (param_names.contains(name)) { ctx.register_symbol_scalar(expr, "m_" + name); @@ -1021,6 +1035,9 @@ auto NumSimMaterialTarget::emit(ConstitutiveModel const &model) const // Register the scalar symbols (state→local, params→m_) into a fresh // emit context — shared by every per-output render below. auto register_scalars = [&](CodeGenContext &c) { + for (auto const &n : reserved_names) { + c.reserve_name(n); + } for (auto const &[name, expr] : model.scalar_symbol_map()) { if (param_names.contains(name)) { c.register_symbol_scalar(expr, "m_" + name); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f0aa040..8479552 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -84,12 +84,17 @@ set(GENERATED_PIECEWISE_T2S_HEADER # Verification spine (#90): nonlinear-stress consistent-tangent FD compile-check. set(GENERATED_TANGENT_HEADER ${CMAKE_CURRENT_BINARY_DIR}/generated/TangentCheck.h) +# Issue #8: user symbols named like generated identifiers (t0/t1 CSE temps, +# T0/T1 template parameters) — compiling this header IS the regression gate. +set(GENERATED_RESERVED_HEADER + ${CMAKE_CURRENT_BINARY_DIR}/generated/ReservedNamesCheck.h) add_custom_command( OUTPUT ${GENERATED_HEADER} ${GENERATED_HARDENING_HEADER} ${GENERATED_NEWTON_HEADER} ${GENERATED_AUTOCAT_HEADER} ${GENERATED_COUPLED_HEADER} ${GENERATED_PIECEWISE_HEADER} ${GENERATED_PIECEWISE_T2S_HEADER} ${GENERATED_TANGENT_HEADER} + ${GENERATED_RESERVED_HEADER} COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/generated COMMAND $ @@ -97,6 +102,7 @@ add_custom_command( ${GENERATED_NEWTON_HEADER} ${GENERATED_AUTOCAT_HEADER} ${GENERATED_COUPLED_HEADER} ${GENERATED_PIECEWISE_HEADER} ${GENERATED_PIECEWISE_T2S_HEADER} ${GENERATED_TANGENT_HEADER} + ${GENERATED_RESERVED_HEADER} DEPENDS generate_compile_check_recipe COMMENT "Generating compile-check headers via numsim-codegen" VERBATIM) @@ -120,7 +126,8 @@ add_executable(compile_check_driver ${GENERATED_COUPLED_HEADER} ${GENERATED_PIECEWISE_HEADER} ${GENERATED_PIECEWISE_T2S_HEADER} - ${GENERATED_TANGENT_HEADER}) + ${GENERATED_TANGENT_HEADER} + ${GENERATED_RESERVED_HEADER}) target_include_directories(compile_check_driver PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/generated ${eigen3_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/support) diff --git a/tests/CodegenContextTest.cpp b/tests/CodegenContextTest.cpp index 0e2f66a..8baa789 100644 --- a/tests/CodegenContextTest.cpp +++ b/tests/CodegenContextTest.cpp @@ -61,4 +61,37 @@ TEST(CodegenContext, ResetClearsStatementsAndCseTable) { EXPECT_EQ(ctx.find(&dummy), nullptr); } +// Issue #8: a user symbol named `t0`/`t1`/… must never be redeclared by a +// generated temporary. reserve_name() makes fresh_name() skip the reserved +// candidates while leaving the numbering of non-colliding temps untouched. +TEST(CodegenContext, FreshNameSkipsReservedNames) { + CodeGenContext ctx; + ctx.reserve_name("t1"); + ctx.reserve_name("t2"); + int a = 0, b = 0, c = 0; + EXPECT_EQ(ctx.emit_temporary(&a, "1.0", "double"), "t0"); + EXPECT_EQ(ctx.emit_temporary(&b, "2.0", "double"), "t3"); + EXPECT_EQ(ctx.emit_temporary(&c, "3.0", "double"), "t4"); +} + +TEST(CodegenContext, ReservationsSurviveReset) { + // reset() keeps reservations (like the symbol registrations they guard): + // the per-block reset() calls inside one emission must not un-reserve the + // function's declared identifiers. + CodeGenContext ctx; + ctx.reserve_name("t0"); + int a = 0, b = 0; + EXPECT_EQ(ctx.emit_temporary(&a, "1.0", "double"), "t1"); + ctx.reset(); + EXPECT_EQ(ctx.emit_temporary(&b, "2.0", "double"), "t2"); +} + +TEST(CodegenContext, FullResetClearsReservations) { + CodeGenContext ctx; + ctx.reserve_name("t0"); + ctx.full_reset(); + int a = 0; + EXPECT_EQ(ctx.emit_temporary(&a, "1.0", "double"), "t0"); +} + } // namespace numsim::codegen diff --git a/tests/RecipeTest.cpp b/tests/RecipeTest.cpp index 9e50e67..cb386f6 100644 --- a/tests/RecipeTest.cpp +++ b/tests/RecipeTest.cpp @@ -242,6 +242,54 @@ TEST(Recipe, EmitComputeFunctionRejectsResidualRecipe) { } } +// Issue #8: user symbols named like generated identifiers (`t0`, `t1`, … +// CSE temporaries; `T0`, `T1`, … template parameters) must not be +// redeclared / shadowed by the generated code. +TEST(Recipe, UserSymbolsNamedLikeGeneratedIdentifiersDoNotCollide) { + ConstitutiveModel m("ReservedNames"); + auto t0 = m.add_scalar_input("t0"); + auto t1 = m.add_scalar_input("t1"); + auto T0 = m.add_tensor_input("T0", 3, 2); + m.add_output("y", t0 * t1 + sin(t0)); + m.add_output("T1", t0 * T0); + + auto src = m.emit_compute_function(); + + // No generated temporary may redeclare the inputs. + EXPECT_EQ(src.find("auto t0 ="), std::string::npos) << "got:\n" << src; + EXPECT_EQ(src.find("auto t1 ="), std::string::npos) << "got:\n" << src; + // Template parameters skip the declared T0 (tensor input) and T1 (tensor + // output): the two tensor arguments get the next free names T2, T3. + EXPECT_EQ(src.find("typename T0"), std::string::npos) << "got:\n" << src; + EXPECT_EQ(src.find("typename T1"), std::string::npos) << "got:\n" << src; + EXPECT_NE(src.find("template "), + std::string::npos) + << "got:\n" << src; + EXPECT_NE(src.find("T2 const &T0"), std::string::npos) << "got:\n" << src; + EXPECT_NE(src.find("T3 &T1_out"), std::string::npos) << "got:\n" << src; +} + +TEST(Recipe, GeneratedNamingUnchangedWithoutCollision) { + // Skip-only-on-collision guarantee: a recipe with no `tN`/`TN`-shaped + // symbols keeps the exact numbering it always had (T0/T1 templates, + // temps starting at t0). + ConstitutiveModel m("NoCollision"); + auto k = m.add_parameter("k", 1.5); + auto x = m.add_scalar_input("x"); + auto eps = m.add_tensor_input("eps", 3, 2); + m.add_output("y", k * x + sin(x)); + m.add_output("sigma", k * eps); + + auto src = m.emit_compute_function(); + + EXPECT_NE(src.find("template "), + std::string::npos) + << "got:\n" << src; + EXPECT_NE(src.find("T0 const &eps"), std::string::npos) << "got:\n" << src; + EXPECT_NE(src.find("T1 &sigma_out"), std::string::npos) << "got:\n" << src; + EXPECT_NE(src.find("auto t0 ="), std::string::npos) << "got:\n" << src; +} + // The shared handle-resolution defends against cross-recipe handle use. TEST(Recipe, ResidualRejectsForeignHandle) { ConstitutiveModel m1("M1"); diff --git a/tests/generated/compile_check_driver.cpp b/tests/generated/compile_check_driver.cpp index 27a381d..eb21cfd 100644 --- a/tests/generated/compile_check_driver.cpp +++ b/tests/generated/compile_check_driver.cpp @@ -13,6 +13,7 @@ #include "NewtonCheck.h" #include "PiecewiseCheck.h" #include "PiecewiseT2sCheck.h" +#include "ReservedNamesCheck.h" #include "TangentCheck.h" #include "numerical_tangent_verifier.h" #include @@ -373,6 +374,33 @@ TEST(CompileCheckGenerated, ConsistentTangentMatchesNumericalDiff) { } } +// Issue #8: the ReservedNamesCheck recipe names every symbol like a GENERATED +// identifier (scalar inputs t0/t1 vs CSE temporaries, tensor input T0 / tensor +// output T1 vs template parameters). That this TU compiles at all is the +// regression gate — before the fix the generated header redeclared t0 and +// shadowed its own template parameter T0. The numeric checks below confirm the +// renamed generated identifiers still wire the right values through. +TEST(CompileCheckGenerated, ReservedUserNamesCompileAndEvaluate) { + double const t0 = 0.5; + double const t1 = 2.0; + tmech::tensor T0; // zero-initialised + T0(0, 0) = 1.0; + T0(1, 2) = 0.25; + + double y_out = 0.0; + tmech::tensor T1_out; + + // Generated signature (registration order): t0, t1, T0, y_out, T1_out. + ReservedNamesCheck_compute(t0, t1, T0, y_out, T1_out); + + // y = t0*t1 + sin(t0) + EXPECT_NEAR(y_out, t0 * t1 + std::sin(t0), 1e-12); + // T1 = (t0 + t1) * T0 + EXPECT_NEAR(T1_out(0, 0), (t0 + t1) * 1.0, 1e-12); + EXPECT_NEAR(T1_out(1, 2), (t0 + t1) * 0.25, 1e-12); + EXPECT_NEAR(T1_out(2, 2), 0.0, 1e-12); +} + // Negative control (review #91 H2): the verifier must have discriminating // power — a correct tangent passes, a deliberately-wrong one FAILS, and the // worst index points at the corrupted component. Without this, a verifier that diff --git a/tests/generated/generate_compile_check_recipe.cpp b/tests/generated/generate_compile_check_recipe.cpp index 3ca0436..33a15f8 100644 --- a/tests/generated/generate_compile_check_recipe.cpp +++ b/tests/generated/generate_compile_check_recipe.cpp @@ -13,6 +13,8 @@ // 6. PiecewiseCheck — tensor_if_then_else emission vs tmech // 7. PiecewiseT2sCheck — tensor_to_scalar_if_then_else (subterm) // 8. TangentCheck — FD consistent-tangent verification (#90) +// 9. ReservedNamesCheck — user symbols named t0/t1/T0/T1 vs generated +// temporaries + template parameters (#8) #include @@ -52,11 +54,12 @@ auto write_single_file(numsim::codegen::ConstitutiveModel const &model, } // namespace int main(int argc, char *argv[]) { - if (argc < 9) { + if (argc < 10) { std::cerr << "usage: " << argv[0] << " " " " - " \n"; + " " + " \n"; return 1; } @@ -223,5 +226,24 @@ int main(int argc, char *argv[]) { if (int rc = write_single_file(model, argv[8]); rc != 0) return rc; } + // ── Recipe 9: RESERVED generated names (issue #8) ──────────────────── + // + // Every symbol is deliberately named like a GENERATED identifier: scalar + // inputs `t0`/`t1` collide with the CSE temporaries, tensor input `T0` + // and tensor output `T1` collide with the template parameters. Before + // the fix the emitted header redeclared `t0` (`auto t0 = ...` next to + // the parameter) and shadowed `T0` (`template (T0 const + // &T0)`) — an uncompilable header. Compiling this header IS the + // regression gate; the driver additionally checks the math. + { + ConstitutiveModel model("ReservedNamesCheck"); + auto t0 = model.add_scalar_input("t0"); + auto t1 = model.add_parameter("t1", 2.0); + auto T0 = model.add_tensor_input("T0", 3, 2); + model.add_output("y", t0 * t1 + sin(t0)); + model.add_output("T1", (t0 + t1) * T0); + if (int rc = write_single_file(model, argv[9]); rc != 0) return rc; + } + return 0; }