Skip to content

Increase the high-end GCC version in qt-ci #3488

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Ferenc-
Copy link
Contributor

@Ferenc- Ferenc- commented May 20, 2025

No description provided.

@louwers louwers requested a review from ntadej May 21, 2025 19:09
@ntadej
Copy link
Collaborator

ntadej commented May 21, 2025

The change is ok, but the test will have to be fixed.

@Ferenc-
Copy link
Contributor Author

Ferenc- commented May 22, 2025

@ntadej Thanks for the quick feedback!

Are we sure that the test is at fault? Because this is not an assertion failure, but simply a call to image.resize({0, 0}); raises an error from STL, because of trying to do a memset into an 0 long buffer.

Either the if statement here should ensure that none of the dimensions is 0:

    void fill(uint8_t value) {
        if (valid()) {

Or perhaps the bool valid() should return false if any of the dimensions is 0.

Fixes the following kind of errors:
```
In function ‘constexpr typename __gnu_cxx::__enable_if<std::__is_byte<_Tp>::__value, void>::__type std::__fill_a1(_Tp*, _Tp*, const _Tp&) [with _Tp = unsigned char]’,
    inlined from ‘constexpr void std::__fill_a(_FIte, _FIte, const _Tp&) [with _FIte = unsigned char*; _Tp = unsigned char]’ at /usr/include/c++/14/bits/stl_algobase.h:998:21,
    inlined from ‘constexpr void std::fill(_ForwardIterator, _ForwardIterator, const _Tp&) [with _ForwardIterator = unsigned char*; _Tp = unsigned char]’ at /usr/include/c++/14/bits/stl_algobase.h:1029:20,
    inlined from ‘void mbgl::Image<Mode>::fill(uint8_t) [with mbgl::ImageAlphaMode Mode = mbgl::ImageAlphaMode::Exclusive]’ at /home/runner/work/maplibre-native/maplibre-native/source/include/mbgl/util/image.hpp:82:22,
    inlined from ‘void mbgl::Image<Mode>::resize(mbgl::Size) [with mbgl::ImageAlphaMode Mode = mbgl::ImageAlphaMode::Exclusive]’ at /home/runner/work/maplibre-native/maplibre-native/source/include/mbgl/util/image.hpp:93:26,
    inlined from ‘virtual void Image_Resize_Test::TestBody()’ at /home/runner/work/maplibre-native/maplibre-native/source/test/util/image.test.cpp:101:17:
/usr/include/c++/14/bits/stl_algobase.h:972:25: error: ‘void* __builtin_memset(void*, int, long unsigned int)’ offset 0 is out of the bounds [0, 0] [-Werror=array-bounds=]
  972 |         __builtin_memset(__first, static_cast<unsigned char>(__tmp), __len);
      |         ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Signed-off-by: Ferenc Géczi <[email protected]>
@Ferenc-
Copy link
Contributor Author

Ferenc- commented May 23, 2025

So I have surmised from the copy function, that when size is empty we can return, so I did the same for fill.
@cgalvan could you check?

@@ -77,7 +77,7 @@ class Image : private util::noncopyable {
size_t bytes() const { return stride() * size.height; }

void fill(uint8_t value) {
if (valid()) {
if (valid() && !size.isEmpty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already implied by valid()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I have missed that. Then this needs another look at, how come isEmpty returns flase, despite the size being {0, 0}.

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.

4 participants