Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector operation improvements #14

Merged
merged 6 commits into from
Sep 12, 2022

Conversation

skitov
Copy link
Contributor

@skitov skitov commented Aug 31, 2022

No description provided.

@skitov skitov force-pushed the vector-operation-improvements branch from b434a42 to 8de945e Compare August 31, 2022 20:50
@igagis
Copy link
Member

igagis commented Sep 1, 2022

CI fails for several reasons.

One of them is this:
https://github.com/cppfw/r4/runs/8122708470?check_suite_focus=true#step:6:103
(see line 102 in the build log)

looks like new version of MSVC compiler is more strict than previous ones, it wasn't complaining about this before. And it is good, because the complain looks valid.

I think the solution to this one would be that there should be no negate() function (and unary minus operator) for vectors of unsigned types. This can be achieved using SFINAE and std::enable_if together with type trait of signedness (std::is_signed).

I think actually this is also why operator-=(vector) had implementation not via unary minus and operator+=(), i.e. in order to support subtraction operation for unsigned types.

Hm, maybe negation still makes some sense for unsigned types? It essentially is a bitwise NOT operation with correction of 1. So, instead of "turning off" the negate() for unsigned types perhaps we need to make a different implementation of it for unsigned types, this can be done using if constexpr (std::is_signed ...){}.

@igagis
Copy link
Member

igagis commented Sep 3, 2022

For the COVERAGE build failure, I have submitted this Issue LouisBrunner/checks-action#35

@igagis
Copy link
Member

igagis commented Sep 3, 2022

I have fixed several MSVS compiler warnings on master, please update the PR

@igagis
Copy link
Member

igagis commented Sep 3, 2022

For Msys2 builds failure, I suspect that there's some problem in compiler, because it also fails on master. In Msys2 they use bleeding edge versions and sometimes there are unfixed bugs, usually they disappear in couple of weeks or so, let's see.

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2022

Codecov Report

Merging #14 (89a9410) into master (c489dcf) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
- Coverage   87.97%   87.92%   -0.06%     
==========================================
  Files           5        5              
  Lines         707      704       -3     
==========================================
- Hits          622      619       -3     
  Misses         85       85              
Impacted Files Coverage Δ
src/r4/vector.hpp 91.53% <100.00%> (-0.11%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@igagis igagis merged commit 0328501 into cppfw:master Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants