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

field.ipp: IWYU for <cstdint> #2676

Closed
wants to merge 1 commit into from

Conversation

ednolan
Copy link
Contributor

@ednolan ednolan commented May 1, 2023

Previously, a file that included only boost/beast/http/fields.hpp would fail to compile on GCC 13.1.0 with the following error:

In file included from /boost/boost/beast/http/field.hpp:411,
                 from /boost/boost/beast/http/fields.hpp:16,
                 from /repro/boost_repro.cpp:1:
/boost/boost/beast/http/impl/field.ipp:30:10: error: 'uint32_t' in namespace 'std' does not name a type; did you mean 'wint_t'?
   30 |     std::uint32_t
      |          ^~~~~~~~
      |          wint_t

This was because boost/beast/http/impl/field.ipp relied on a transitive include that is no longer present in libstdc++ 13.

This commit addresses the issue by adding a <cstdint> include to boost/beast/http/impl/field.ipp.

Previously, a file that included only boost/beast/http/fields.hpp
would fail to compile on GCC 13.1.0 with the following error:

In file included from /boost/boost/beast/http/field.hpp:411,
                 from /boost/boost/beast/http/fields.hpp:16,
                 from /repro/boost_repro.cpp:1:
/boost/boost/beast/http/impl/field.ipp:30:10: error: 'uint32_t' in namespace 'std' does not name a type; did you mean 'wint_t'?
   30 |     std::uint32_t
      |          ^~~~~~~~
      |          wint_t

This was because boost/beast/http/impl/field.ipp relied on a
transitive include that is no longer present in libstdc++ 13.

This commit addresses the issue by adding a <cstdint> include to
boost/beast/http/impl/field.ipp.
@vinniefalco
Copy link
Member

That should have been caught with this unit test:

#include <boost/beast/http/field.hpp>

@ednolan
Copy link
Contributor Author

ednolan commented May 1, 2023

My guess would be that the unit tests were never run with GCC 13. I don't see that compiler in the list of CI checks

@vinniefalco
Copy link
Member

of COURSE... the ONE configuration that we don't test... is the one that has the error :)

@ednolan
Copy link
Contributor Author

ednolan commented May 1, 2023

To be fair, GCC 13 only came out five days ago.

@vinniefalco
Copy link
Member

Oh! I thought it was older.. thanks

@royjacobson
Copy link

Just had the same problem with beast/core/file_base.hpp. It might be a wider problem :/

@vinniefalco
Copy link
Member

we need to add gcc13 to the matrix and fix all the errors

@klemens-morgenstern
Copy link
Collaborator

Can confirm from local install (fedora 38)

@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

Merging #2676 (283781e) into develop (b0996f0) will decrease coverage by 0.03%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2676      +/-   ##
===========================================
- Coverage    92.96%   92.94%   -0.03%     
===========================================
  Files          177      177              
  Lines        13651    13658       +7     
===========================================
+ Hits         12691    12694       +3     
- Misses         960      964       +4     
Impacted Files Coverage Δ
include/boost/beast/http/impl/field.ipp 97.64% <ø> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0996f0...283781e. Read the comment docs.

@klemens-morgenstern
Copy link
Collaborator

Completed in #2682

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.

5 participants