Skip to content

[ntuple][RFC] Do type-check in RValue::GetRef with basic data types #18719

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

ferdymercury
Copy link
Collaborator

@ferdymercury ferdymercury commented May 14, 2025

This Pull request:

Changes or fixes:

Fixes #18316

Checklist:

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

@ferdymercury ferdymercury marked this pull request as ready for review May 14, 2025 15:24
@ferdymercury ferdymercury requested a review from jblomer as a code owner May 14, 2025 15:24
@ferdymercury ferdymercury changed the title [ntuple] Do type-check in RValue::GetRef with basic data types [ntuple][RFC] Do type-check in RValue::GetRef with basic data types May 14, 2025
Copy link

Test Results

    19 files      19 suites   3d 18h 32m 36s ⏱️
 2 745 tests  2 724 ✅ 0 💤 21 ❌
50 673 runs  50 640 ✅ 0 💤 33 ❌

For more details on these failures, see this check.

Results for commit d186288.

Copy link
Contributor

@jblomer jblomer 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 need to go through RField<T> for the type name check, like RField<T>::TypeName() == fField->GetTypeName(). This will still not solve the case when the fField pointer is already gone at the time we call GetRef(). While we could store the full type name string in RValue, that would be costly and I'd like to avoid it.

I think this issue needs more thought and discussion.

@ferdymercury ferdymercury marked this pull request as draft May 15, 2025 21:17
@ferdymercury ferdymercury added this to the 6.38.00 milestone May 15, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ntuple] RValue::GetRef<T> should type-check
2 participants