Skip to content

Conversation

@guitargeek
Copy link
Contributor

The mechanism to strip away the "self" argument from the call to a static method checks the type of "self", matching the exact type.

However, it should also remove the "self" if it is an instance of a derived type, because in C++ it is also possible to call static methods via instances of derived types.

A unit test was also added.

Closes #20463.

Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

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

Fine for me if all tests pass.

The mechanism to strip away the "self" argument from the call to a
static method checks the type of "self", matching the exact type.

However, it should also remove the "self" if it is an instance of a
derived type, because in C++ it is also possible to call static methods
via instances of derived types.

A unit test was also added.

Closes root-project#20463.
@guitargeek
Copy link
Contributor Author

Tests passed (just fixed typo in the commit message with recent push)

@guitargeek guitargeek merged commit f30e8fd into root-project:master Nov 24, 2025
28 of 29 checks passed
@guitargeek guitargeek deleted the issue-20463 branch November 24, 2025 21:02
@guitargeek guitargeek changed the title [CPyCppyy] Correcly handle static method calls via derived instance [CPyCppyy] Correctly handle static method calls via derived instance Nov 24, 2025
@wlav
Copy link
Contributor

wlav commented Nov 24, 2025

I'd recommend leaving ((CPPInstance*)arg0)->ObjectIsA() == GetScope()) in place (with an or for the subtype check). Will be faster in the most common cases.

@guitargeek
Copy link
Contributor Author

guitargeek commented Nov 24, 2025

Good point, I was thinking that too before. But the equality shortcut check is already done in the implementation of Cppyy::IsSubtype:
https://github.com/root-project/root/blob/master/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx#L1615

And I don't think the overhead of the function call matters here, or does it?

@wlav
Copy link
Contributor

wlav commented Nov 24, 2025

No, you're quite right, that's good enough. It's also a better place for the short-cut.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

pyROOT TH1::GetDefaultSumw2 throws error when called

3 participants