Skip to content

[ntuple] Use dynamic platform-dependent width rather than hard-coded width of long #19429

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

Merged
merged 2 commits into from
Jul 24, 2025

Conversation

ferdymercury
Copy link
Collaborator

This Pull request:

Changes or fixes:

Fixes #11616

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@jblomer jblomer self-assigned this Jul 22, 2025
@ferdymercury ferdymercury marked this pull request as ready for review July 22, 2025 10:55
@ferdymercury ferdymercury requested a review from jblomer as a code owner July 22, 2025 10:55
Copy link

github-actions bot commented Jul 22, 2025

Test Results

    21 files      21 suites   3d 9h 36m 20s ⏱️
 3 217 tests  3 217 ✅ 0 💤 0 ❌
65 834 runs  65 834 ✅ 0 💤 0 ❌

Results for commit d6a99b5.

♻️ This comment has been updated with latest results.

Copy link
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

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

I think we should generally replace all instances of this pattern (ie also for kInt_t, kUInt_t etc.) and fully rely on RField to do the mapping.

@ferdymercury
Copy link
Collaborator Author

I think we should generally replace all instances of this pattern (ie also for kInt_t, kUInt_t etc.) and fully rely on RField to do the mapping.

Do you mean eg changing

case kUShort_t: Attach(std::make_unique<RField<uint16_t>>("_0")); break;

With

case kUShort_t: Attach(std::make_unique<RField<UShort_t>>("_0")); break;

?

Rather than copy-pasting the mapping to stdint done in RTypesCore.h here, fully delegate on RField the conversion, so that we are agnostic about what Long_t, Long64_t represents.

as suggested by hahnjo
@ferdymercury ferdymercury requested a review from hahnjo July 23, 2025 11:04
@jblomer jblomer merged commit 560f6ff into root-project:master Jul 24, 2025
26 checks passed
@ferdymercury ferdymercury deleted the patch-1 branch July 24, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ntuple] Check the usage of Long_t in RField (size of the type might differ between 32- and 64-bit platforms)
4 participants