Skip to content

Use size_t for offset#18992

Merged
aaronj0 merged 1 commit intoroot-project:masterfrom
ellert:size_t-offset
Jun 10, 2025
Merged

Use size_t for offset#18992
aaronj0 merged 1 commit intoroot-project:masterfrom
ellert:size_t-offset

Conversation

@ellert
Copy link
Copy Markdown
Contributor

@ellert ellert commented Jun 7, 2025

Offsets should use size_t. Using int causes test failures on Big Endian (s390x).

[ RUN      ] VariableReflectionTest.StaticConstExprDatamember
/builddir/build/BUILD/root-6.36.00/interpreter/CppInterOp/unittests/CppInterOp/VariableReflectionTest.cpp:545: Failure
Expected equality of these values:
  3
  *(int*)offset
    Which is: 0
/builddir/build/BUILD/root-6.36.00/interpreter/CppInterOp/unittests/CppInterOp/VariableReflectionTest.cpp:561: Failure
Expected equality of these values:
  5
  *(int*)offset
    Which is: 0
/builddir/build/BUILD/root-6.36.00/interpreter/CppInterOp/unittests/CppInterOp/VariableReflectionTest.cpp:580: Failure
Expected equality of these values:
  2
  *(int*)offset
    Which is: 0
[  FAILED  ] VariableReflectionTest.StaticConstExprDatamember (9 ms)

[ RUN      ] VariableReflectionTest.StaticConstExprDatamember
/builddir/build/BUILD/root-6.36.00/interpreter/CppInterOp/unittests/CppInterOp/VariableReflectionTest.cpp:545: Failure
Expected equality of these values:
  3
  *(int*)offset
    Which is: 0
/builddir/build/BUILD/root-6.36.00/interpreter/CppInterOp/unittests/CppInterOp/VariableReflectionTest.cpp:561: Failure
Expected equality of these values:
  5
  *(int*)offset
    Which is: 0
/builddir/build/BUILD/root-6.36.00/interpreter/CppInterOp/unittests/CppInterOp/VariableReflectionTest.cpp:580: Failure
Expected equality of these values:
  2
  *(int*)offset
    Which is: 0
[  FAILED  ] VariableReflectionTest.StaticConstExprDatamember (9 ms)
@ellert ellert requested a review from dpiparo as a code owner June 7, 2025 18:15
@guitargeek guitargeek requested review from aaronj0 and removed request for dpiparo June 7, 2025 19:15
@guitargeek guitargeek requested a review from Vipul-Cariappa June 7, 2025 19:15
@ferdymercury
Copy link
Copy Markdown
Collaborator

Is this related to #14512 ?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 8, 2025

Test Results

    19 files      19 suites   3d 7h 56m 5s ⏱️
 2 805 tests  2 805 ✅ 0 💤 0 ❌
51 795 runs  51 795 ✅ 0 💤 0 ❌

Results for commit 7bfd27a.

Copy link
Copy Markdown
Contributor

@aaronj0 aaronj0 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for this useful patch, which will be upstreamed to CppInterOp

aaronj0 added a commit to aaronj0/CppInterOp that referenced this pull request Jun 10, 2025
This fixes failures on 32 bit platforms. Upstream of root-project/root#18992
aaronj0 added a commit to aaronj0/CppInterOp that referenced this pull request Jun 10, 2025
This fixes failures on 32 bit platforms. Upstream of root-project/root#18992
@aaronj0 aaronj0 merged commit 360c9b1 into root-project:master Jun 10, 2025
24 of 25 checks passed
aaronj0 added a commit to compiler-research/CppInterOp that referenced this pull request Jun 10, 2025
This fixes failures on 32 bit platforms. Upstream of root-project/root#18992
@hahnjo
Copy link
Copy Markdown
Member

hahnjo commented Jun 12, 2025

For the moment, this broke cppinterop-diff. Can this be fixed?

@ellert ellert deleted the size_t-offset branch June 19, 2025 12:39
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