fix: fix compile error on macOS with clang 21#237
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes several macOS clang 21 compilation issues by resolving type-mismatch/override problems and ensuring template calls select the intended overloads in the blob/avro format code paths.
Changes:
- Fix
std::mintype deduction issues by explicitly selectinguint64_tand casting touint32_twhereInputStream::Readexpectsuint32_t. - Fix
MemoryPoolImpl::Reallocsignature to correctly overrideMemoryPool::Reallocon platforms wheresize_t!=uint64_t. - Reorder
Blob::FromPathdefinitions soBlob::Implis complete when used withstd::make_unique.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/paimon/format/blob/blob_format_writer.cpp | Makes read-chunk sizing compile-clean by disambiguating std::min and casting for Read(...). |
| src/paimon/format/avro/avro_input_stream_impl.cpp | Adjusts Read(...) sizing to match uint32_t API requirements. |
| src/paimon/common/memory/memory_pool.cpp | Fixes Realloc override signature for cross-platform compilation. |
| src/paimon/common/data/blob.cpp | Moves FromPath implementations below Blob::Impl definition to avoid incomplete-type issues. |
Comments suppressed due to low confidence (1)
src/paimon/common/memory/memory_pool.cpp:62
total_allocated_sizeis anatomic<int64_t>, butnew_size - old_sizeis computed insize_t(unsigned). When shrinking, this relies on unsigned underflow and implementation-defined conversion to produce a negative delta, which is brittle and can break accounting or trigger compiler diagnostics. Compute the delta in a signed type explicitly (e.g.,int64_t delta = static_cast<int64_t>(new_size) - static_cast<int64_t>(old_size)) before callingfetch_add(same applies to the similarfetch_add(new_size - old_size)in the aligned branch).
void* MemoryPoolImpl::Realloc(void* p, size_t old_size, size_t new_size, uint64_t alignment) {
if (alignment == 0) {
void* memptr = ::realloc(p, new_size);
total_allocated_size.fetch_add(new_size - old_size);
max_allocated.store(std::max(total_allocated_size.load(), max_allocated.load()));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Thank you, @lxy-9602! |
Contributor
|
@wgtmac Hi, can you compile passed on Mac? |
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
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.
Purpose
Compiling paimon-cpp on macOS with clang 21 has some minor issues.
Tests
N/A
API and Format
N/A
Documentation
N/A
Generative AI tooling
No