PDF: const-qualify scalar by-value constructor params#576
Merged
Conversation
Add top-level const to the cheap scalar by-value parameters `n` (ExponentialFunction) and `bits_per_sample` (SampledFunction), matching the codebase convention of const-qualifying scalar params in definitions. Object/vector params stay non-const because they are std::move'd into members, so adding const would defeat the move. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqx1dUpDUykRxui8FYwxvM
Extend #576's convention to the two constructors defined inline in pdf_document_element.hpp: add top-level const to the cheap copied-not- moved by-value params `width` (Iterator, CodeRange) and `codes` (CodeRange), mirroring the existing BitReader(const std::string_view data, const std::size_t byte_offset) in pdf_image.cpp. Top-level const on a value param does not change the function type, so callers and ABI are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uqx1dUpDUykRxui8FYwxvM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Summary
Follow-up const-consistency sweep across the
pdfmodule. The codebaseconvention is to const-qualify cheap scalar by-value parameters in
definitions; this adds the two stragglers that were missing it:
ExponentialFunction(..., const double n)SampledFunction(..., const std::int32_t bits_per_sample, ...)Why the diff is small
Guiding rule: what can be const without hurting performance (move) should
be const. A full scan of the module found that nearly every object/vector
by-value parameter is
std::move'd into a member (or otherwise mutated), soconst-qualifying them would defeat the move — those correctly stay non-const.
The genuinely missing cases were just these two scalar constructor params.
One other candidate (
uint_to_code'svalue) is shifted in-place, so italso correctly remains non-const.
Testing
cmake --build cmake-build-relwithdebinfo --target odr— builds clean.