Skip to content

THRIFT-5785: fix boost include#3431

Merged
kpumuk merged 1 commit into
apache:masterfrom
CJCombrink:remove_boost_numeric_cast
May 1, 2026
Merged

THRIFT-5785: fix boost include#3431
kpumuk merged 1 commit into
apache:masterfrom
CJCombrink:remove_boost_numeric_cast

Conversation

@CJCombrink
Copy link
Copy Markdown
Contributor

@CJCombrink CJCombrink commented Apr 30, 2026

Remove another boost header include from the public API

  • It is only used in THeaderTransport.cpp so move it there without complicating it
  • It throws a TTransportException so it should not be used in other code
  • People that depends on this can use boost::numeric_cast or std::in_range (C++20) instead
  • The usage in the unit test was not needed since std::string::size() returns an unsigned long on 64-bit compilers
  • Did you create an Apache Jira ticket? THRIFT-5785
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

The following tickets are releated

- It is only used in THeaderTransport.cpp so move it there without compicating it
- It throws a TTransportException so it should not be used in other code
- People that depends on this can use boost::numeric_cast or std::in_range instead
- The usage in the unit test was not needed since std::string::size() returns an unsigned long on 64 compilers
@CJCombrink
Copy link
Copy Markdown
Contributor Author

PS: just rewriting the offending function without using boost was more complex than I have anticipated (see the linked tickets)
On closer inspection I realised that just moving the function internal removes the problem thus no complex code changes needed

@mergeable mergeable Bot added the c++ Pull requests that update C++ code label Apr 30, 2026
Copy link
Copy Markdown
Member

@emmenlau emmenlau left a comment

Choose a reason for hiding this comment

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

Looks good to me. I did not check test results. If tests are green, please merge!

@hdu-sdlzx
Copy link
Copy Markdown

lgtm
I can build my project with installed thrift and without boost now.

@kpumuk kpumuk merged commit 8970c23 into apache:master May 1, 2026
89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Pull requests that update C++ code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants