Skip to content

Conversation

@dpiparo
Copy link
Member

@dpiparo dpiparo commented Apr 21, 2025

Instead of all data members the type of which has a name beginning with "TRef".

Fixes ROOT-7052

This PR builds on top of the solid work done here #14930.

@dpiparo dpiparo self-assigned this Apr 21, 2025
@dpiparo dpiparo requested a review from pcanal as a code owner April 21, 2025 17:31
@pcanal pcanal added this to the 6.38.00 milestone Apr 21, 2025
@ferdymercury
Copy link
Collaborator

ferdymercury commented Apr 21, 2025

Thanks a lot! Optional idea: could you add and updates fClassObject. on line 287 ?

@github-actions
Copy link

github-actions bot commented Apr 21, 2025

Test Results

    18 files      18 suites   4d 4h 30m 21s ⏱️
 2 731 tests  2 731 ✅ 0 💤 0 ❌
47 768 runs  47 768 ✅ 0 💤 0 ❌

Results for commit 62a9c3a.

♻️ This comment has been updated with latest results.

@dpiparo
Copy link
Member Author

dpiparo commented Apr 22, 2025

Yes, I focussed on the functional change before. I'll make that part of the new version of the PR after the review.

@ferdymercury
Copy link
Collaborator

@olifre Do you happen to still have a reproducer/test for this issue of yours with a class CBTRef deriving from TRef, that you could share?

@olifre
Copy link
Contributor

olifre commented Apr 25, 2025

@olifre Do you happen to still have a reproducer/test for this issue of yours with a class CBTRef deriving from TRef, that you could share?

Sadly, I have no example at hand 😢 . As far as I remember, I think I tried this in the context of a larger framework with classes inheriting from TRef and then did not continue to create a minimal reproducer, sorry.

Instead of all data members the type of which has a name beginning
with "TRef".

Fixes ROOT-7052
@dpiparo
Copy link
Member Author

dpiparo commented May 1, 2025

Just added a test.

" int i;\n"
"ClassDef(Foo2, 1);\n"
"};"
"class TRefFoo {int i;};\n"
Copy link
Member

Choose a reason for hiding this comment

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

Why the name TRefFoo?

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind this the point of the test :)

Suggested change
"class TRefFoo {int i;};\n"
"// We are testing that this is not confused for a ROOT TRef or TRefArray
"class TRefFoo {int i;};\n"

Copy link
Member Author

Choose a reason for hiding this comment

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

:)

Copy link
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

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

Thanks.

@dpiparo dpiparo merged commit cdc439c into root-project:master May 2, 2025
38 of 41 checks passed
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.

4 participants