diff --git a/docs/workflow.md b/docs/workflow.md index 262ab15..abf1862 100644 --- a/docs/workflow.md +++ b/docs/workflow.md @@ -64,14 +64,17 @@ sequenceDiagram Model->>PM: run(pctx) PM->>SVP: run(pctx) + SVP->>SVP: reset m_state_variables_non_empty = false
(round-3 safe-on-throw guarantee) SVP->>SVP: walk symbols, check identifier validity
(ASCII range + C++ keyword reject) SVP->>SVP: walk outputs, check no undeclared symbols - SVP-->>PM: postconditions:
{symbols-declared, identifiers-valid} + SVP->>SVP: verify_state_variable_symbol_alignment
(Phase 2.1+ pairing invariant) + SVP->>SVP: set m_state_variables_non_empty
= !state_variables().empty() + SVP-->>PM: postconditions:
{symbols-declared, identifiers-valid,
state-variables-checked,
state-variables-non-empty (iff non-empty)} PM->>TSP: run(pctx) TSP->>TSP: cross-check Role.is_symmetric vs
tensor_space.perm (Skew = conflict) TSP->>TSP: cross-check Role.expected_rank vs
tensor.rank() - TSP-->>PM: postconditions:
{tensor-space-validated} + TSP-->>PM: postconditions:
{tensor-space-declarations-checked} PM->>CEP: run(pctx) Note over CEP: Cycle-break via
unique_ptr indirection diff --git a/include/numsim_codegen/passes/code_emit_pass.h b/include/numsim_codegen/passes/code_emit_pass.h index 0743a3a..2cfcb49 100644 --- a/include/numsim_codegen/passes/code_emit_pass.h +++ b/include/numsim_codegen/passes/code_emit_pass.h @@ -21,8 +21,17 @@ namespace numsim::codegen { // // Preconditions: `pass_tags::symbols_declared` + // `pass_tags::identifiers_valid` + `pass_tags::tensor_space_declarations_checked` -// (i.e. SymbolValidationPass + TensorSpaceConsistencyPass must have run -// first). If you add a pass that transforms expressions (e.g. a future +// + `pass_tags::state_variables_checked` (i.e. SymbolValidationPass + +// TensorSpaceConsistencyPass must have run first). The state-var tag +// (PR #66 round-3 review #3) codifies the bypass-caveat from +// `verify_state_variable_symbol_alignment` in the precondition graph: +// SymbolValidationPass advertises it unconditionally, so any emit +// pipeline transitively requires SVP — a custom pipeline that stubs +// the identifier tags via a no-op pass and skips SVP would now fail +// at `run()` rather than silently emit code with an unverified +// state-var alignment. +// +// If you add a pass that transforms expressions (e.g. a future // TimeIntegrationPass), register it AFTER the validators but BEFORE // CodeEmitPass. class CodeEmitPass final : public Pass { @@ -33,7 +42,8 @@ class CodeEmitPass final : public Pass { [[nodiscard]] auto preconditions() const -> std::vector override { return {pass_tags::symbols_declared, pass_tags::identifiers_valid, - pass_tags::tensor_space_declarations_checked}; + pass_tags::tensor_space_declarations_checked, + pass_tags::state_variables_checked}; } [[nodiscard]] auto postconditions() const -> std::vector override { diff --git a/include/numsim_codegen/passes/pass.h b/include/numsim_codegen/passes/pass.h index 046080c..08d9fae 100644 --- a/include/numsim_codegen/passes/pass.h +++ b/include/numsim_codegen/passes/pass.h @@ -15,6 +15,7 @@ namespace numsim::codegen { struct SymbolDecl; +struct StateVariable; // Shared state for a single PassManager invocation. Passes read the // recipe via `model` (a RecipeView — const-only today, will gain a @@ -73,6 +74,20 @@ enum class LookupError { std::string const &name) noexcept -> std::expected; +// Convenience: resolve a name to its StateVariable record. Returns +// nullable pointer rather than `std::expected` per `docs/workflow.md` +// §6.1 — only one failure mode ("no state variable with this name") +// that the caller could distinguish. Linear scan over +// `pctx.model.state_variables()`; state-variable counts are small +// (single-digit typical, tens worst-case for multi-surface plasticity) +// so the scan is fine. Could be promoted to a populated map in +// PassContext if that ever changes. Issue #59 / REVIEW-pr-58.md m3. +// +// Definition lives in recipe.h where `StateVariable` is complete. +[[nodiscard]] inline auto find_state_variable_by_name( + PassContext const &pctx, std::string_view name) noexcept + -> StateVariable const *; + // Abstract base for a single codegen pass. // // Passes advertise their pre/postconditions as string tags. PassManager @@ -88,10 +103,34 @@ class Pass { public: virtual ~Pass() = default; [[nodiscard]] virtual auto name() const -> std::string_view = 0; + + // Tags this pass requires to be satisfied before `run()` is called. + // Stable across the pass's lifetime — `preconditions()` is queried by + // `PassManager::run()` *before* `run()`, so the value must not depend + // on per-call state. [[nodiscard]] virtual auto preconditions() const -> std::vector { return {}; } + + // Tags this pass advertises as satisfied after `run()` returns + // successfully. **Lifecycle (PR #66 round-2 review #11, round-3 #2):** + // `PassManager::run()` queries `postconditions()` AFTER each call to + // `run()`, never before. Implementations are therefore free to return + // values that depend on per-call state set during `run()`. + // + // Most passes (`TensorSpaceConsistencyPass`, `CodeEmitPass`) return + // literal initialiser lists from `postconditions()` and don't need to + // think about lifecycle. `SymbolValidationPass` uses the pattern to + // conditionally advertise `state_variables_non_empty` based on the + // recipe — see its `run()` body for the canonical shape. + // + // **Guidance for passes that DO store per-call postcondition state:** + // reset that state at the *start* of `run()`, not the end, so a + // throwing `run()` leaves the pass in a coherent observable state and + // a pre-`run()` query reports the safe-default shape. This is + // convention, not a framework-enforced contract — PassManager has no + // way to check it. [[nodiscard]] virtual auto postconditions() const -> std::vector { return {}; diff --git a/include/numsim_codegen/passes/pass_tags.h b/include/numsim_codegen/passes/pass_tags.h index f6844a1..1959bd9 100644 --- a/include/numsim_codegen/passes/pass_tags.h +++ b/include/numsim_codegen/passes/pass_tags.h @@ -36,6 +36,38 @@ namespace numsim::codegen::pass_tags { inline constexpr std::string_view symbols_declared = "symbols-declared"; inline constexpr std::string_view identifiers_valid = "identifiers-valid"; +// State-variable tags advertised by `SymbolValidationPass` (issue #59 / +// REVIEW-pr-58.md m2, refined by PR #66 review). +// +// Phase 2.2+ passes that touch state variables fall in one of two +// shapes, and the framework can serve them differently with two tags: +// +// * `state_variables_checked` — ALWAYS advertised. The +// SymbolValidationPass body has walked `state_variables()` and +// verified the alignment invariant with `symbols()` (via +// `verify_state_variable_symbol_alignment`). A pass that runs even +// on pure-elasticity recipes (e.g. one that emits no-op stubs for +// consistency) takes this as a precondition. +// +// * `state_variables_non_empty` — advertised IFF the recipe actually +// has at least one state variable. A pass whose body is a no-op on +// empty state vectors (TimeIntegrationPass, KuhnTuckerLoweringPass) +// takes this as a precondition; on a pure-elasticity recipe +// PassManager then fails at `run()` with a clear +// `"pass X requires precondition state-variables-non-empty but no +// earlier pass advertised that postcondition"` message — surfacing +// the misconfiguration loudly rather than the pass silently doing +// nothing. (PassManager checks preconditions at `run()` time, not +// at registration; see `pass_manager.h:39-46`.) +// +// The split keeps the framework's tag-tracking honest: a passcondition +// has the same name across every consumer, so a typo or rename surfaces +// at compile time, not in the form of a pass that quietly never fires. +inline constexpr std::string_view state_variables_checked = + "state-variables-checked"; +inline constexpr std::string_view state_variables_non_empty = + "state-variables-non-empty"; + // TensorSpaceConsistencyPass postcondition. Renamed from the original // `tensor-space-validated` per P6 — the original overpromised; Phase 2's // expression-level inference pass will use a separate tag. diff --git a/include/numsim_codegen/passes/symbol_validation_pass.h b/include/numsim_codegen/passes/symbol_validation_pass.h index 5724bfe..651f1e1 100644 --- a/include/numsim_codegen/passes/symbol_validation_pass.h +++ b/include/numsim_codegen/passes/symbol_validation_pass.h @@ -34,7 +34,17 @@ class SymbolValidationPass final : public Pass { } [[nodiscard]] auto postconditions() const -> std::vector override { - return {pass_tags::symbols_declared, pass_tags::identifiers_valid}; + // `state_variables_non_empty` is conditionally advertised — PR #66 + // review #6. `m_state_variables_non_empty` is set by `run()` to + // reflect the recipe's actual state-variable count; PassManager + // queries postconditions() AFTER run(), so the conditional is safe. + std::vector tags{pass_tags::symbols_declared, + pass_tags::identifiers_valid, + pass_tags::state_variables_checked}; + if (m_state_variables_non_empty) { + tags.push_back(pass_tags::state_variables_non_empty); + } + return tags; } void run(PassContext &pctx) override; // defined in recipe.h after class. @@ -111,6 +121,13 @@ class SymbolValidationPass final : public Pass { } return !is_cxx_keyword(s); } + +private: + // Runtime flag set by `run()` to drive the conditional + // `state_variables_non_empty` postcondition (declared above). + // Default-false so a pre-`run()` query reports the safe shape (no + // such query exists today, but defensive). + bool m_state_variables_non_empty = false; }; } // namespace numsim::codegen diff --git a/include/numsim_codegen/recipe.h b/include/numsim_codegen/recipe.h index a4a7650..52a1ab4 100644 --- a/include/numsim_codegen/recipe.h +++ b/include/numsim_codegen/recipe.h @@ -190,6 +190,14 @@ struct OutputDecl { Role role = roles::Other; }; +// Forward-declared in a sub-namespace so `ConstitutiveModel`'s +// `friend struct testing_detail::state_variable_alignment_access;` +// resolves. Definition lives only in the test TU (see +// `tests/StateVariableTest.cpp`). PR #66 round-2 review #5. +namespace testing_detail { +struct state_variable_alignment_access; +} // namespace testing_detail + // The constitutive-model registry. Holds declared inputs, parameters, // outputs, and their semantic role tags. Target-agnostic — the same // recipe can be emitted as standalone C++, MOOSE Material, Abaqus UMAT, @@ -488,6 +496,27 @@ class ConstitutiveModel { } private: + // ─── Test-only friends ────────────────────────────────────────── + // + // PR #66 round-2 review #5: grants negative-path tests in `tests/` + // controlled write access to `m_symbols` and `m_state_variables` so + // they can simulate the corruption shapes Phase 2.2+ mutating passes + // might produce. Placed under the `private:` label (rather than the + // public block) so a reader scanning the class layout doesn't mistake + // the line for a public-API affordance. + // + // Lives in the `testing_detail::` sub-namespace — both as a marker + // ("if you're writing production code and naming this, you've taken + // a wrong turn") and to keep the friend's reachable surface narrow + // from production-namespace TUs. + // + // The struct is declared here but never defined inside + // `numsim::codegen`; tests provide the only definition (see + // `tests/StateVariableTest.cpp`). Keeping the surface to a single + // friend struct means a single ODR slot in the test binary; if a + // future test TU needs the same access, factor the definition to a + // shared `tests/internal/` header rather than redefining it. + friend struct testing_detail::state_variable_alignment_access; // Phase 2.1 (M1+M2 in REVIEW-pr-58.md): reject a duplicate symbol // name at add time, with a clear pipeline-misconfiguration message. // Two collision modes this catches: @@ -634,6 +663,107 @@ inline auto RecipeView::tensor_symbol_map() const -> TensorSymbolMap const & { return &decl; } +[[nodiscard]] inline auto find_state_variable_by_name( + PassContext const &pctx, std::string_view name) noexcept + -> StateVariable const * { + for (auto const &sv : pctx.model.state_variables()) { + if (sv.name == name) { + return &sv; + } + } + return nullptr; +} + +// Phase 2.2 prep (issue #59 / REVIEW-pr-58.md m1 + PR #66 review): +// verify the structural alignment between `model.state_variables()` and +// `model.symbols()`. Throws `std::runtime_error` naming the specific +// mismatch — silent corruption is worse than a loud throw. +// +// Today no public `ConstitutiveModel` API can produce a violation +// because `add_*_state_variable` writes both vectors atomically. The +// check earns its keep when Phase 2.2+ mutating passes start touching +// `m_state_variables` / `m_symbols` independently — at that point this +// function lives inside `SymbolValidationPass`, so any pipeline that +// registers `SymbolValidationPass` re-verifies the invariant on every +// run. Phase 2.2 mutating passes can re-run the pass (or a derived +// `StructuralIntegrityPass` that calls this) to re-check between +// mutations. +// +// **Bypass caveat:** a caller that constructs a `PassManager` and +// omits `SymbolValidationPass` skips the check entirely. The framework +// has no enforcement against that today; the convention is that any +// pipeline that emits code or mutates state variables registers +// `SymbolValidationPass` first. `ConstitutiveModel::validate()` and +// `::emit_compute_function()` both do so. +// +// TODO(phase-2.2): this scaffolding fault-injects the assertion arms +// (see the friend-poison tests in `tests/StateVariableTest.cpp`). When +// the first mutating pass lands, mutator-driven integration tests +// (apply mutator → assert throws) will be added alongside this corpus, +// not replace it — the fault-injection harness covers the assertion +// arms; the integration tests cover the mutator-to-arm wiring. +inline void verify_state_variable_symbol_alignment(RecipeView model) { + // Hoisted out of the lambda body (PR #66 round-3 #6): the span is + // identical across all `check()` invocations within a single call + // — fetching once makes that explicit and removes a redundant copy + // per state variable. + auto const symbols = model.symbols(); + auto check = [&](std::size_t idx, std::string_view expected_name, + SymbolDecl::Category expected_cat, + StateVariable const &sv, char const *which) { + if (idx >= symbols.size()) { + throw std::runtime_error(std::format( + "ConstitutiveModel '{}': StateVariable '{}' carries {} index " + "{} which is out of bounds for symbols() (size {}). State " + "variable / symbol vectors are out of sync.", + model.name(), sv.name, which, idx, symbols.size())); + } + auto const &s = symbols[idx]; + if (s.name != expected_name) { + throw std::runtime_error(std::format( + "ConstitutiveModel '{}': StateVariable '{}' {} index points " + "to SymbolDecl named '{}'; expected '{}' (derived from " + "StateVariable.name). The symbols() entry has been renamed or " + "the indices have shifted.", + model.name(), sv.name, which, s.name, expected_name)); + } + if (s.category != expected_cat) { + throw std::runtime_error(std::format( + "ConstitutiveModel '{}': StateVariable '{}' {} symbol has " + "wrong Category (expected={}, got={}). State variable / symbol " + "vectors are out of sync.", + model.name(), sv.name, which, + static_cast(expected_cat), + static_cast(s.category))); + } + if (s.kind != sv.kind) { + throw std::runtime_error(std::format( + "ConstitutiveModel '{}': StateVariable '{}' {} symbol has Kind " + "mismatched against the state-variable record (expected={}, " + "got={}).", + model.name(), sv.name, which, + static_cast(sv.kind), + static_cast(s.kind))); + } + if (sv.kind == SymbolDecl::Kind::Tensor && + (s.dim != sv.dim || s.rank != sv.rank)) { + throw std::runtime_error(std::format( + "ConstitutiveModel '{}': StateVariable '{}' {} symbol has " + "dim/rank ({}, {}) mismatched against the state-variable " + "record ({}, {}).", + model.name(), sv.name, which, s.dim, s.rank, sv.dim, sv.rank)); + } + }; + for (auto const &sv : model.state_variables()) { + check(sv.current_symbol_idx, sv.name, + SymbolDecl::Category::StateVariableCurrent, sv, "current"); + auto const old_name = + std::string{sv.name} + std::string{state_variable_old_suffix}; + check(sv.old_symbol_idx, old_name, + SymbolDecl::Category::StateVariableOld, sv, "old"); + } +} + // ─── Function-frame rendering (free functions) ─────────────────────── // // These render the surrounding function frame after the body statements @@ -750,6 +880,22 @@ inline auto render_compute_function( inline void SymbolValidationPass::run(PassContext &pctx) { auto const &model = pctx.model; + // Reset the conditional-postcondition flag on entry — PR #66 round-2 + // review #2. Two failure modes leak prior state without this: + // (a) `verify_state_variable_symbol_alignment` below throws → the + // end-of-run assignment never executes → the flag retains its + // previous value; + // (b) the same pass instance is reused across two recipes with + // different state-variable counts → state advertised between + // calls reflects the prior recipe. + // (A pre-`run()` query on a pristine pass instance is already covered + // by the default-initialiser at `symbol_validation_pass.h:130` — the + // reset-at-entry here does not need to address that case.) + // Resetting up-front guarantees `m_state_variables_non_empty` always + // reflects the *current* run; the real value is computed after + // validation succeeds at the bottom of this function. + m_state_variables_non_empty = false; + // P4 (post-fixup C1): build the name → index lookup once. Indices // survive any future `m_symbols.push_back()` reallocation, whereas // the original SymbolDecl pointers would silently dangle the first @@ -845,6 +991,18 @@ inline void SymbolValidationPass::run(PassContext &pctx) { "before referencing a symbol in an output expression."; throw std::runtime_error(msg); } + + // (3) State-variable alignment invariant (issue #59 / PR #66 review). + // Throws if any StateVariable's `current_symbol_idx` / `old_symbol_idx` + // points at a mismatched SymbolDecl. Today's public API can't produce + // a violation, but running the check here means Phase 2.2+ mutating + // passes that touch state variables can re-run SymbolValidationPass + // to re-verify between stages — the invariant isn't bypassable by + // running passes outside `ConstitutiveModel::validate()`. + verify_state_variable_symbol_alignment(model); + + // Drive the conditional `state_variables_non_empty` postcondition. + m_state_variables_non_empty = !model.state_variables().empty(); } inline void TensorSpaceConsistencyPass::run(PassContext &pctx) { diff --git a/tests/StateVariableTest.cpp b/tests/StateVariableTest.cpp index 882dab6..579bf4c 100644 --- a/tests/StateVariableTest.cpp +++ b/tests/StateVariableTest.cpp @@ -15,9 +15,14 @@ // flow through codegen. This file only exercises the IR layer. #include +#include #include #include +// Shared friend-access header for negative-path tests — see header +// comment for the ODR rationale (PR #66 round-3 review #7). +#include "internal/state_variable_alignment_access.h" + #include #include #include @@ -316,4 +321,311 @@ TEST(StateVariable, StateVarUsableInOutputExpression) { << "_old value appears as const input param: " << src; } +// ─── Phase 2.2 prep (issue #59) ────────────────────────────────────── + +TEST(StateVariablePhase22Prep, AlignmentInvariantValidatesScalarAndTensor) { + // Item 1 / REVIEW-pr-58.md m1: validate() runs the + // validate_state_variable_symbol_alignment() invariant check before + // the pass pipeline. A well-formed recipe with both scalar and tensor + // state variables (interleaved with inputs + parameters so indices + // land at non-trivial offsets) must pass. + ConstitutiveModel model("M"); + auto K = model.add_parameter("K", 1.0); + (void)model.add_scalar_input("eps_v"); + auto alpha = model.add_scalar_state_variable( + "alpha", cas::make_expression(0.0)); + (void)K; + (void)alpha; + auto eps_p_init = cas::make_expression(3, 2); + (void)model.add_tensor_state_variable("eps_p", 3, 2, eps_p_init); + + EXPECT_NO_THROW(model.validate()); + + // Indices land where add_*_state_variable claims they do: + for (auto const &sv : model.state_variables()) { + auto const &cur = model.symbols()[sv.current_symbol_idx]; + auto const &old = model.symbols()[sv.old_symbol_idx]; + EXPECT_EQ(cur.name, sv.name); + EXPECT_EQ(old.name, sv.name + "_old"); + EXPECT_EQ(cur.kind, sv.kind); + EXPECT_EQ(old.kind, sv.kind); + EXPECT_EQ(cur.category, SymbolDecl::Category::StateVariableCurrent); + EXPECT_EQ(old.category, SymbolDecl::Category::StateVariableOld); + if (sv.kind == SymbolDecl::Kind::Tensor) { + EXPECT_EQ(cur.dim, sv.dim); + EXPECT_EQ(cur.rank, sv.rank); + EXPECT_EQ(old.dim, sv.dim); + EXPECT_EQ(old.rank, sv.rank); + } + } +} + +TEST(StateVariablePhase22Prep, SymbolValidationPassAdvertisesStateVarTags) { + // PR #66 review: tag split. SymbolValidationPass advertises + // `state_variables_checked` unconditionally and + // `state_variables_non_empty` only when the recipe actually has + // state variables. PassManager queries postconditions() AFTER run(), + // so the conditional advertisement is observable through it. + auto has = [](auto const &v, std::string_view t) { + for (auto const &x : v) { + if (x == t) return true; + } + return false; + }; + + // PR #66 round-2 #6: pin the pre-`run()` contract. A pristine pass + // instance reports the safe (no non_empty) shape. Without this test + // a future refactor that lazy-inits in `run()` could silently regress + // the safe default. + { + SymbolValidationPass pristine; + auto const pre_run = pristine.postconditions(); + EXPECT_TRUE(has(pre_run, pass_tags::state_variables_checked)); + EXPECT_FALSE(has(pre_run, pass_tags::state_variables_non_empty)) + << "non_empty tag must NOT fire before run()"; + } + + // Pure-elasticity recipe: only the always-on tag advertised. + { + ConstitutiveModel pure_elastic("E"); + (void)pure_elastic.add_parameter("mu", 0.5); + PassContext pctx{RecipeView{pure_elastic}, CodeGenContext{}, + std::nullopt, {}}; + SymbolValidationPass pass; + pass.run(pctx); + auto const post = pass.postconditions(); + EXPECT_TRUE(has(post, pass_tags::state_variables_checked)); + EXPECT_FALSE(has(post, pass_tags::state_variables_non_empty)) + << "non_empty tag must NOT fire for state-var-free recipes"; + // validate() itself must still succeed: + EXPECT_NO_THROW(pure_elastic.validate()); + } + + // Recipe with at least one state variable: both tags advertised. + { + ConstitutiveModel hardening("H"); + (void)hardening.add_scalar_state_variable( + "alpha", cas::make_expression(0.0)); + PassContext pctx{RecipeView{hardening}, CodeGenContext{}, + std::nullopt, {}}; + SymbolValidationPass pass; + pass.run(pctx); + auto const post = pass.postconditions(); + EXPECT_TRUE(has(post, pass_tags::state_variables_checked)); + EXPECT_TRUE(has(post, pass_tags::state_variables_non_empty)); + } + + // Same-instance reuse: PR #66 round-3 #5. Mode (b) of the reset + // comment in `SymbolValidationPass::run` ("same instance reused + // across recipes with different state-var counts") is exercised + // here. Without the reset-at-entry the second `run()` would leave + // the non-empty tag advertised from the first run. + { + SymbolValidationPass reused; + + ConstitutiveModel with_sv("S"); + (void)with_sv.add_scalar_state_variable( + "alpha", cas::make_expression(0.0)); + ConstitutiveModel pure("P"); + (void)pure.add_parameter("k", 1.0); + + // First run: state-var recipe → both tags advertised. + { + PassContext pctx1{RecipeView{with_sv}, CodeGenContext{}, + std::nullopt, {}}; + reused.run(pctx1); + auto const post1 = reused.postconditions(); + EXPECT_TRUE(has(post1, pass_tags::state_variables_checked)); + EXPECT_TRUE(has(post1, pass_tags::state_variables_non_empty)); + } + + // Second run on the SAME instance: pure-elasticity recipe → + // non-empty tag must be cleared. + { + PassContext pctx2{RecipeView{pure}, CodeGenContext{}, + std::nullopt, {}}; + reused.run(pctx2); + auto const post2 = reused.postconditions(); + EXPECT_TRUE(has(post2, pass_tags::state_variables_checked)); + EXPECT_FALSE(has(post2, pass_tags::state_variables_non_empty)) + << "reset-at-entry must clear stale non-empty advert " + "when the same pass instance is reused across recipes " + "with different state-var counts (mode b)"; + } + + // Reverse order on the SAME instance for symmetry — pure first, + // then state-var. Catches a hypothetical "lazy-init that only + // initialises once" regression. + { + PassContext pctx3{RecipeView{pure}, CodeGenContext{}, + std::nullopt, {}}; + reused.run(pctx3); + EXPECT_FALSE(has(reused.postconditions(), + pass_tags::state_variables_non_empty)); + PassContext pctx4{RecipeView{with_sv}, CodeGenContext{}, + std::nullopt, {}}; + reused.run(pctx4); + EXPECT_TRUE(has(reused.postconditions(), + pass_tags::state_variables_non_empty)); + } + } +} + +TEST(StateVariablePhase22Prep, FindStateVariableByName) { + // Item 3 / REVIEW-pr-58.md m3: find_state_variable_by_name resolves + // a name to its StateVariable record. Returns nullptr on miss. + ConstitutiveModel model("M"); + (void)model.add_scalar_state_variable( + "alpha", cas::make_expression(0.0)); + auto eps_p_init = cas::make_expression(3, 2); + (void)model.add_tensor_state_variable("eps_p", 3, 2, eps_p_init); + + PassContext pctx{RecipeView{model}, CodeGenContext{}, std::nullopt, {}}; + + auto const *alpha_sv = find_state_variable_by_name(pctx, "alpha"); + ASSERT_NE(alpha_sv, nullptr); + EXPECT_EQ(alpha_sv->name, "alpha"); + EXPECT_EQ(alpha_sv->kind, SymbolDecl::Kind::Scalar); + + auto const *eps_p_sv = find_state_variable_by_name(pctx, "eps_p"); + ASSERT_NE(eps_p_sv, nullptr); + EXPECT_EQ(eps_p_sv->name, "eps_p"); + EXPECT_EQ(eps_p_sv->kind, SymbolDecl::Kind::Tensor); + + // The paired `_old` symbol is NOT itself a StateVariable record — it + // is a SymbolDecl whose owning StateVariable is named without the + // suffix. Looking up by the suffixed name must miss. + EXPECT_EQ(find_state_variable_by_name(pctx, "alpha_old"), nullptr); + + // Unrelated names miss. + EXPECT_EQ(find_state_variable_by_name(pctx, "nope"), nullptr); + EXPECT_EQ(find_state_variable_by_name(pctx, ""), nullptr); +} + +// ─── Negative-path tests for the alignment invariant ──────────────── +// +// `verify_state_variable_symbol_alignment` has 5 distinct throw branches. +// The public ConstitutiveModel API cannot violate the invariant today, +// so the only way to verify each branch fires correctly is to mutate +// the internals directly via the `testing_detail::state_variable_alignment_access` +// friend struct. Its definition lives in `tests/internal/` (PR #66 +// round-3 #7) so future test TUs that need the same access include the +// shared header rather than redefining the struct — making the ODR +// contract mechanical, not social. + +namespace { +// Anonymous-namespace alias (PR #66 round-3 #8) so a future test TU +// using `using poison = ...` for a different access struct doesn't +// quietly mean different things across TUs. +using sv_align_poison = + testing_detail::state_variable_alignment_access; +} // namespace + +TEST(StateVariablePhase22Prep, AlignmentDetectsCorruptionOutOfBoundsIdx) { + // PR #66 round-2 #7: single try/catch + FAIL() — silent test-pass on + // non-throw is impossible. Negative-substring asserts pin which arm + // fires (round-2 #8). + ConstitutiveModel model("M"); + (void)model.add_scalar_state_variable( + "alpha", cas::make_expression(0.0)); + sv_align_poison::poison_state_var_current_idx(model, 0, 999); + try { + model.validate(); + FAIL() << "expected std::runtime_error from alignment OOB arm"; + } catch (std::runtime_error const &e) { + std::string const what = e.what(); + EXPECT_NE(what.find("out of bounds"), std::string::npos) << what; + EXPECT_NE(what.find("alpha"), std::string::npos) << what; + // Pin: no other arm should have fired. + EXPECT_EQ(what.find("wrong Category"), std::string::npos) << what; + EXPECT_EQ(what.find("Kind mismatched"), std::string::npos) << what; + EXPECT_EQ(what.find("dim/rank"), std::string::npos) << what; + } +} + +TEST(StateVariablePhase22Prep, AlignmentDetectsCorruptionRenamedSymbol) { + ConstitutiveModel model("M"); + (void)model.add_scalar_state_variable( + "alpha", cas::make_expression(0.0)); + auto const cur_idx = model.state_variables()[0].current_symbol_idx; + sv_align_poison::poison_symbol_name(model, cur_idx, "beta"); + try { + model.validate(); + FAIL() << "expected std::runtime_error from alignment name arm"; + } catch (std::runtime_error const &e) { + std::string const what = e.what(); + EXPECT_NE(what.find("named 'beta'"), std::string::npos) << what; + EXPECT_NE(what.find("expected 'alpha'"), std::string::npos) << what; + EXPECT_EQ(what.find("wrong Category"), std::string::npos) << what; + EXPECT_EQ(what.find("Kind mismatched"), std::string::npos) << what; + EXPECT_EQ(what.find("dim/rank"), std::string::npos) << what; + EXPECT_EQ(what.find("out of bounds"), std::string::npos) << what; + } +} + +TEST(StateVariablePhase22Prep, AlignmentDetectsCorruptionWrongCategory) { + ConstitutiveModel model("M"); + (void)model.add_scalar_state_variable( + "alpha", cas::make_expression(0.0)); + auto const cur_idx = model.state_variables()[0].current_symbol_idx; + sv_align_poison::poison_symbol_category(model, cur_idx, + SymbolDecl::Category::Input); + try { + model.validate(); + FAIL() << "expected std::runtime_error from alignment category arm"; + } catch (std::runtime_error const &e) { + std::string const what = e.what(); + // The message must print both observed and expected enum values + // (PR #66 round-1 #4). + EXPECT_NE(what.find("wrong Category"), std::string::npos) << what; + EXPECT_NE(what.find("expected="), std::string::npos) << what; + EXPECT_NE(what.find("got="), std::string::npos) << what; + EXPECT_EQ(what.find("Kind mismatched"), std::string::npos) << what; + EXPECT_EQ(what.find("dim/rank"), std::string::npos) << what; + EXPECT_EQ(what.find("out of bounds"), std::string::npos) << what; + } +} + +TEST(StateVariablePhase22Prep, AlignmentDetectsCorruptionWrongKind) { + ConstitutiveModel model("M"); + (void)model.add_scalar_state_variable( + "alpha", cas::make_expression(0.0)); + auto const cur_idx = model.state_variables()[0].current_symbol_idx; + sv_align_poison::poison_symbol_kind(model, cur_idx, SymbolDecl::Kind::Tensor); + try { + model.validate(); + FAIL() << "expected std::runtime_error from alignment kind arm"; + } catch (std::runtime_error const &e) { + std::string const what = e.what(); + EXPECT_NE(what.find("Kind mismatched"), std::string::npos) << what; + EXPECT_NE(what.find("expected="), std::string::npos) << what; + EXPECT_NE(what.find("got="), std::string::npos) << what; + // Pin: must be the kind arm, not the dim arm — the dim arm is + // gated on `sv.kind == Tensor` (record side, still Scalar here); + // a future reorder that swapped gating could silently still pass. + EXPECT_EQ(what.find("dim/rank"), std::string::npos) << what; + EXPECT_EQ(what.find("wrong Category"), std::string::npos) << what; + EXPECT_EQ(what.find("out of bounds"), std::string::npos) << what; + } +} + +TEST(StateVariablePhase22Prep, AlignmentDetectsCorruptionWrongDim) { + ConstitutiveModel model("M"); + auto eps_p_init = cas::make_expression(3, 2); + (void)model.add_tensor_state_variable("eps_p", 3, 2, eps_p_init); + auto const cur_idx = model.state_variables()[0].current_symbol_idx; + sv_align_poison::poison_symbol_dim(model, cur_idx, 2); + try { + model.validate(); + FAIL() << "expected std::runtime_error from alignment dim arm"; + } catch (std::runtime_error const &e) { + std::string const what = e.what(); + EXPECT_NE(what.find("dim/rank"), std::string::npos) << what; + EXPECT_NE(what.find("eps_p"), std::string::npos) << what; + EXPECT_EQ(what.find("wrong Category"), std::string::npos) << what; + EXPECT_EQ(what.find("Kind mismatched"), std::string::npos) << what; + EXPECT_EQ(what.find("out of bounds"), std::string::npos) << what; + } +} + } // namespace numsim::codegen diff --git a/tests/internal/state_variable_alignment_access.h b/tests/internal/state_variable_alignment_access.h new file mode 100644 index 0000000..f1f9449 --- /dev/null +++ b/tests/internal/state_variable_alignment_access.h @@ -0,0 +1,68 @@ +#ifndef NUMSIM_CODEGEN_TESTS_INTERNAL_STATE_VARIABLE_ALIGNMENT_ACCESS_H +#define NUMSIM_CODEGEN_TESTS_INTERNAL_STATE_VARIABLE_ALIGNMENT_ACCESS_H + +// Test-only friend access to `ConstitutiveModel`'s private members for +// negative-path tests of the StateVariable ↔ SymbolDecl alignment +// invariant. PR #66 round-3 review #7. +// +// **Why this header exists.** `ConstitutiveModel` grants `friend` to +// `numsim::codegen::testing_detail::state_variable_alignment_access`. +// If two test TUs each defined that struct independently — even with +// the same name and same body — the program would be ODR-violating +// ([basic.def.odr]/12: at most one definition of a non-inline class). +// Factoring the definition here gives the test binary a single ODR +// slot; future test TUs that need the same access `#include` this +// header rather than redefining the struct. +// +// Methods are `static` so the struct itself is never instantiated; +// each method is `inline` so multiple TU includes link cleanly. +// Production code paths cannot accidentally rely on this — the header +// lives under `tests/internal/` and isn't on the public include path. + +#include + +#include +#include +#include + +namespace numsim::codegen::testing_detail { + +struct state_variable_alignment_access { + // Clobber the name of the SymbolDecl pointed at by a state variable's + // current/old index. Simulates a mutating pass renaming a symbol + // without keeping the pair in sync. + static inline void poison_symbol_name(ConstitutiveModel &m, + std::size_t idx, + std::string new_name) { + m.m_symbols[idx].name = std::move(new_name); + } + // Clobber the category of a symbol. + static inline void poison_symbol_category(ConstitutiveModel &m, + std::size_t idx, + SymbolDecl::Category new_cat) { + m.m_symbols[idx].category = new_cat; + } + // Clobber the kind of a symbol. + static inline void poison_symbol_kind(ConstitutiveModel &m, + std::size_t idx, + SymbolDecl::Kind new_kind) { + m.m_symbols[idx].kind = new_kind; + } + // Clobber the dim of a tensor symbol. + static inline void poison_symbol_dim(ConstitutiveModel &m, + std::size_t idx, + std::size_t new_dim) { + m.m_symbols[idx].dim = new_dim; + } + // Push the current_symbol_idx of a state variable past the end of + // m_symbols to simulate a vector-shrink that left stale indices. + static inline void poison_state_var_current_idx(ConstitutiveModel &m, + std::size_t sv_idx, + std::size_t new_idx) { + m.m_state_variables[sv_idx].current_symbol_idx = new_idx; + } +}; + +} // namespace numsim::codegen::testing_detail + +#endif // NUMSIM_CODEGEN_TESTS_INTERNAL_STATE_VARIABLE_ALIGNMENT_ACCESS_H