Skip to content

Fix: Add missing include needed for gcc v15 and clang v19#2313

Merged
galabovaa merged 1 commit intoERGO-Code:latestfrom
alexdewar:add-missing-include
May 1, 2025
Merged

Fix: Add missing include needed for gcc v15 and clang v19#2313
galabovaa merged 1 commit intoERGO-Code:latestfrom
alexdewar:add-missing-include

Conversation

@alexdewar
Copy link
Copy Markdown
Contributor

There is a missing include for cstdint. Without it, the build fails on gcc v15 and clang v19 (see below).

Gcc v15 has already been shipped in some distros (Fedora, Arch, maybe others), so it would be great if you could tag a new release ASAP. I'm using HiGHS indirectly via the Rust crate and currently can't build it on my machine.

Build log:

In file included from /home/alex/code/HiGHS/extern/filereaderlp/reader.cpp:20:
/home/alex/code/HiGHS/src/../extern/zstr/zstr.hpp:58:24: error: use of undeclared identifier 'uintptr_t'; did you mean 'intptr_t'?
   58 |         std::to_string(uintptr_t(zstrm_p->next_in)) +
      |                        ^
/usr/include/unistd.h:267:20: note: 'intptr_t' declared here
  267 | typedef __intptr_t intptr_t;
      |                    ^
In file included from /home/alex/code/HiGHS/extern/filereaderlp/reader.cpp:20:
/home/alex/code/HiGHS/src/../extern/zstr/zstr.hpp:59:41: error: use of undeclared identifier 'uintptr_t'; did you mean 'intptr_t'?
   59 |         ", avail_in: " + std::to_string(uintptr_t(zstrm_p->avail_in)) +
      |                                         ^
/usr/include/unistd.h:267:20: note: 'intptr_t' declared here
  267 | typedef __intptr_t intptr_t;
      |                    ^
In file included from /home/alex/code/HiGHS/extern/filereaderlp/reader.cpp:20:
/home/alex/code/HiGHS/src/../extern/zstr/zstr.hpp:60:41: error: use of undeclared identifier 'uintptr_t'; did you mean 'intptr_t'?
   60 |         ", next_out: " + std::to_string(uintptr_t(zstrm_p->next_out)) +
      |                                         ^
/usr/include/unistd.h:267:20: note: 'intptr_t' declared here
  267 | typedef __intptr_t intptr_t;
      |                    ^
In file included from /home/alex/code/HiGHS/extern/filereaderlp/reader.cpp:20:
/home/alex/code/HiGHS/src/../extern/zstr/zstr.hpp:61:42: error: use of undeclared identifier 'uintptr_t'; did you mean 'intptr_t'?
   61 |         ", avail_out: " + std::to_string(uintptr_t(zstrm_p->avail_out)) + ")";
      |                                          ^
/usr/include/unistd.h:267:20: note: 'intptr_t' declared here
  267 | typedef __intptr_t intptr_t;
      |                    ^
In file included from /home/alex/code/HiGHS/extern/filereaderlp/reader.cpp:20:
/home/alex/code/HiGHS/src/../extern/zstr/zstr.hpp:194:31: error: use of undeclared identifier 'uint32_t'
  194 |           zstrm_p->avail_in = uint32_t(in_buff_end - in_buff_start);
      |                               ^
/home/alex/code/HiGHS/src/../extern/zstr/zstr.hpp:198:15: error: use of undeclared identifier 'uint32_t'
  198 |               uint32_t((out_buff.get() + buff_size) - out_buff_free_start);
      |               ^
/home/alex/code/HiGHS/src/../extern/zstr/zstr.hpp:266:28: error: use of undeclared identifier 'uint32_t'
  266 |       zstrm_p->avail_out = uint32_t(buff_size);
      |                            ^
/home/alex/code/HiGHS/src/../extern/zstr/zstr.hpp:305:25: error: use of undeclared identifier 'uint32_t'
  305 |     zstrm_p->avail_in = uint32_t(pptr() - pbase());
      |                         ^
8 errors generated.

There is a missing include for cstdint. Without it, the build fails on
gcc v15 and clang v19.
@jajhall
Copy link
Copy Markdown
Member

jajhall commented May 1, 2025

Are you happy to release this as 1.10.1 @galabovaa ?

@alexdewar
Copy link
Copy Markdown
Contributor Author

Update: I have been able to get it to build with clang v19 on Alpine Linux instead (I was using Arch before). I think it may be that clang was using gcc's libstdc++ rather than its own libc++.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.93%. Comparing base (5f28825) to head (2403f07).
Report is 391 commits behind head on latest.

Additional details and impacted files
@@           Coverage Diff           @@
##           latest    #2313   +/-   ##
=======================================
  Coverage   78.93%   78.93%           
=======================================
  Files         343      343           
  Lines       83419    83419           
=======================================
+ Hits        65849    65850    +1     
+ Misses      17570    17569    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@galabovaa galabovaa changed the base branch from master to latest May 1, 2025 11:04
@galabovaa
Copy link
Copy Markdown
Contributor

This is in zstr which is external from HiGHS. We have 1.0.5 from three years ago and I see now at https://github.com/mateidavid/zstr that they are at 1.0.7 tagged in December 2024.

The particular include line was added there only four days ago.

I would rather use their next tagged release than the current version of zstr/latest for the update in HiGHS. So, for now, I will merge this into latest and use it in the next release.

@jajhall
Copy link
Copy Markdown
Member

jajhall commented May 1, 2025

Thanks @alexdewar , we can avoid the patch release, then

@galabovaa galabovaa merged commit c6c824c into ERGO-Code:latest May 1, 2025
119 checks passed
@alexdewar
Copy link
Copy Markdown
Contributor Author

Thanks for being so responsive 😄

@alexdewar
Copy link
Copy Markdown
Contributor Author

I'm wondering if making a patch release might still be appropriate, unless we know that a new version of zstr with the fix will be released imminently. Currently, there are many Linux users who will be unable to compile HiGHS with their system compiler without patching the HiGHS source code in some way.

@galabovaa
Copy link
Copy Markdown
Contributor

We are preparing to make a release soon anyway and this fix will be included.

I tried the version of zstr from latest, but I get a memory leak in our testing, so will certainly wait until they can make a release.

Our patched one should be out relatively soon, I guess in a week or two.

@alexdewar
Copy link
Copy Markdown
Contributor Author

Ok, that's not so bad. Thanks for the update.

chrjabs added a commit to chrjabs/highs-sys that referenced this pull request May 15, 2025
chrjabs added a commit to chrjabs/scuttle that referenced this pull request May 15, 2025
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