Fix #369: drop noexcept from allocating paths - #436
Merged
Merged
Conversation
bad_alloc on these paths called std::terminate instead of propagating: - expression ==, !=, <, like_term_of and the equals/less_than_same_type hooks (they compute hashes lazily, which allocates) - update_hash_value overrides that hash children (n_ary_tree's also allocates a vector) - n_ary_tree/n_ary_vector copy and forwarding ctors, n_ary_vector default ctor, push_back, reserve, insert_hash - symbol_base(name, ...) (copies the name), get_scalar_zero/one (static init allocates), symbol_table::has (builds a std::string) - scalar_expr_less and scalar_pretty_printer comparators, printer print_sequence, latex print_unary, preserve_unary (copies a space), tensor_data_make_imp::evaluate_imp (make_unique), tensor-with-scalar and tensor-with-t2s mul dispatches (build expressions) Moves that copied now move and keep noexcept: tensor_add, tensor_mul and simple_outer_product transfer the space; permute_indices_wrapper moves its base instead of copying it; scalar move-assignment moves the name. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
n_ary_tree/n_ary_vector's (self&&, Args&&...) ctors promised noexcept while forwarding Args into base_t. They are now noexcept(is_nothrow_constructible_v<base_t, Args...>); the sized tensor_expression ctors they call are marked noexcept since they only store two size_t and default-init the rest, so node types keep their nothrow move. static_asserts cover scalar_mul, tensor_mul, tensor_to_scalar_add and both expression_holder instantiations. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #369.
noexcepton paths that can allocate turnedstd::bad_allocintostd::terminate. Audited everynoexceptininclude/andsrc/(287 sites).Removed (can allocate or throw):
expression::operator==,!=,<,like_term_of(+n_ary_tree,tensor_scalar_muloverrides),equals_same_type/less_than_same_type(+visitor_baseoverrides): all compute child hashes lazily.update_hash_valueoverrides that hash children:n_ary_tree(also allocates a vector),unary_op,binary_op,ternary_opand theirupdate_hashfunctors,tensor_scalar_mul,tensor_pow,permute_indices_wrapper,inner_product_wrapper,outer_product_wrapper,tensor_eigenprojection,tensor_eigenvector,tensor_isotropic_function,tensor_inner_product_to_scalar,tensor_to_scalar_divided_difference,tensor_to_scalar_eigenvalue,tensor_to_scalar_scalar_wrapper. (Signature lines only; Fix #372: reset m_hash_value before recomputing in every hash override #431 edits some of these bodies.)n_ary_tree: copy and forwarding ctors.n_ary_vector: default ctor (reserve), copy and forwarding ctors,push_back,reserve,insert_hash.symbol_base(name, ...)(copies the name),get_scalar_zero/one(static init allocates),symbol_table::has(builds astd::string).scalar_expr_less,scalar_pretty_printer;printer_base::print_sequence,latex_printer_base::print_unary;structural_propagation::preserve_unary(copies a space);tensor_data_make_imp::evaluate_imp(make_unique); the tensor-with-scalar and tensor-with-t2s muldispatchfunctions (build expressions).Moves that copied — now genuine moves,
noexceptkept:tensor_add,tensor_mul,simple_outer_productmove the space instead of copying it (droppingnoexceptthere trips clang-tidyperformance-noexcept-move-constructor);permute_indices_wrappermoved viastatic_cast<base>(a copy), nowstatic_cast<base &&>;scalarmove-assignment moves the name.Kept: move ctors/assignments that only move members, getters,
is_same,set_coeff(shared_ptr assignment),set_space/clear_space,scalar_constant::update_hash_value(hashes ascalar_number, no children),scalar_number::abs,printer_base::begin/endandtensor_data::print(stream insertion), fixed-size tmechevaluate_imps, index arithmetic helpers, conditionalnoexcept(noexcept(tag_invoke(...)))CPOs, and the never-instantiatedstatic_assertevaluator fallbacks.Tests —
CoreBugFix.AllocatingPathsAreNotNoexcept:static_asserts that==,<,like_term_of,n_ary_vector::push_backandget_scalar_zero()are notnoexcept; thatscalar_add,tensor_add,tensor_to_scalar_mul,permute_indices_wrapperstay nothrow-move-constructible.tensor_addkeeps itsSymmetricspace.Negative control: compiling the test against
main's sources fails on the five "not noexcept" asserts; removing the space transfer fromtensor_add's move ctor fails the runtime check.Full suite 2347/2347 (gcc-14 Debug); clang-tidy-18 clean on the changed
.cppfiles; clang-format-18 clean.Review fix (eb5ce92). The forwarding move ctors
n_ary_tree(n_ary_tree&&, Args&&...)andn_ary_vector(n_ary_vector&&, Args&&...)kept unconditionalnoexceptwhile their copy twins lost it — same over-promise shape, sinceArgsis forwarded intobase_t. Both are nownoexcept(std::is_nothrow_constructible_v<base_t, Args...>). To keep that condition true for the tensor nodes,tensor_expression(dim, rank)andtensor_expression(tensor_expression&&, dim, rank)are markednoexcept— they only store twosize_tand default-init the remaining members. Plain move ctors stay unconditionallynoexcept; addedstatic_asserts forscalar_mul,tensor_mul,tensor_to_scalar_addand bothexpression_holderinstantiations, so container nothrow-move is pinned.Two observations left as-is (out of scope):
scalar_expr_lessinnumsim_cas_type_traits.his dead code —expr_ordered_mapisstd::map<T, T>with the defaultstd::less, and the struct has no other reference; andscalar::operator=(scalar&&)has no callers (scalaralso pulls inusing base_t::operator=).Full suite 2347/2347 after the review fix; clang-tidy-18 and clang-format-18 clean.