Repository navigation
Conversation
Rename parameters and locals that shadow members, other parameters or globals: - xstrided_view and xdynamic_view bring shape(), strides() and layout() into scope with using-declarations, so GCC flags constructor and stepper parameters with those names. The constructors now take view_shape, view_strides and view_layout, and stepper_begin and stepper_end take iteration_shape. - xreducer::element named its local stepper after the stepper member type. It is now reducer_stepper. - reset_buffer in xarray_adaptor and xtensor_adaptor took a parameter named pointer, like the member type. It is now buffer. - The xio printers took a precision parameter, which GCC 13 reports against any global named precision (the xio test data defines one). The value is the upper bound that init() lowers to the precision the data needs, so it is now max_precision. - The xkeep_slice and xdrop_slice initializer-list constructors used t for both the list and the lambda parameter. The lambda now takes index. - XTENSOR_TRY caught the exception as e, which shadows any e in the calling scope, such as the xfunction lambda parameters. The macro now uses xtensor_check_error.
- test_xarray_adaptor and test_xtensor_adaptor redeclared vec_type with the same definition as the namespace-scope alias. Drop the local copies. - test_xbuffer_adaptor reused owner_adaptor for an adaptor with a different element type and allocator. Name it size_checked_adaptor. - test_xmath named its missing value missing, which shadows the xt::missing placeholder. Name it missing_value. - The is_arr lambdas in test_xreducer named their parameter c, like the tensor in the enclosing test. They only ever receive shapes, so the parameter is now shape.
CHECK_RESULT_TYPE, CHECK_TAG_TYPE and CHECK_TYPE declared a local alias named result_type. TEST_VALUE_HAS_VALUE declared its own result_type for the expected type and then passed it to CHECK_RESULT_TYPE, where the macro's alias shadowed it. The assertion compared the reducer value type with itself and could not fail. The macros now name their alias actual_type, and TEST_VALUE_HAS_VALUE names its alias expected_type. The value_has_value tests now check that sum() returns double for plain arrays and xoptional<double> for optional inputs. They pass.
- argsort_iter only reads data_end in assertions. Mark it [[maybe_unused]] and drop the (void) idx_end cast, which was dead code since idx_end is passed to std::iota. - The flat argmin and argmax overloads declared a value_type alias they never used. Remove it. - initializer_shape does not read its argument when the shape has no dimensions. Mark it [[maybe_unused]]. - accumulate_impl wrapped f in a lambda that was never called for an empty tuple. Fold over the assignments directly. - optional_bitwise combined bool flags with &. The operands are values that are already computed, so && gives the same result. It is also correct for flag types other than bool, where & could clear a set flag (2 & true is 0). The simd path keeps & for batch_bool. - Remove a stray ';' in xreducer_temporary_type<xoptional>. - index_mapper was declared as a struct and defined as a class. Declare it as a class.
Several tests declared aliases that were used only in some builds, or
not used at all. Where the alias pointed at something worth checking,
the test now checks it:
- The xassign_traits aliases in test_xcomplex, test_xmath,
test_xstrided_view and test_xview were only read under
XTENSOR_USE_XSIMD. Without xsimd, simd_linear_assign() is always
false, so the tests now assert that in an #else branch. The
assign_traits2 check in test_xview expects false in both builds and
no longer needs the #if. Note that test_xtensor_core_lib never
defines XTENSOR_USE_XSIMD, even in an xsimd build.
- test_xview declared carr, a dynamic-rank column-major array, without
using it. Add the check the column-major is_contiguous_view comment
describes: such a view is not contiguous because implicit trailing
all() slices may follow.
- test_xexpression only declared the strides aliases of a shared
xfunction, one of them under a garbled name. Check that all four
come from the shape type, as an xfunction has no strides of its own.
Also fix the remaining clang warnings in tests:
- test_xmath tested math::abs(char(1)). Plain char is unsigned on
aarch64, so the line did not test a signed type there. Use
signed char with a negative value.
- test_xexpression_holder moved a temporary into the holder, which
prevents copy elision. Move a named holder instead, which still
exercises the move constructor.
- test_xsort passed the axis as {0}. Pass 0.
Contributor
Merging this PR will degrade performance by 18.92%2 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
|
This branch has not been deployed
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.
Checklist
Description
This PR fixes the
-Wshadowwarnings and the remaining-Wall -Wextra -Wpedanticwarnings that are not conversion or flow-sensitive ones (-Wimplicit-fallthrough,-Wparentheses, unused typedefs and parameters,-Wextra-semi,-Wmismatched-tags,-Wbitwise-instead-of-logical,-Wpessimizing-move,-Wabsolute-value,-Wbraced-scalar-init). Parameters and locals that shadow something get a clearer name. Nothing is silenced with pragmas.Warning sites in scope:
The count is also 0 with
XTENSOR_USE_XSIMD(GCC 15) andXTENSOR_ENABLE_ASSERT(GCC 13, clang 20).Two warnings pointed at library bugs, fixed separately: the
argsortfallthrough (#2954) andxoptional_assembly_storage::data()(#2955). Two more were test bugs, fixed here:test_xreducer, the type-check macros shadowed the expected type, soCHECK_RESULT_TYPEcompared a type with itself and could never fail;math::abs(char(1))does not test a signed type wherecharis unsigned (aarch64); it now usessigned charwith -1.Unused test aliases became assertions where they named something worth checking.