Skip to content
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

[PyROOT] Disable automatic conversion of regular to smart pointers #15125

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Apr 4, 2024

The automatic conversion of ordinary obejcts to smart pointers is disabled for PyROOT because it can cause trouble with overload resolution. If a function has overloads for both ordinary objects and smart pointers, then the implicit conversion to smart pointers can result in the smart pointer overload being hit, even though there would be an overload for the regular object. Since PyROOT didn't have this feature before 6.32 anyway, disabling it was the safest option.

Closes #15117.

A regression test with the reproducer from the GitHub issue was added to roottest:
root-project/roottest#1102

The automatic conversion of ordinary obejcts to smart pointers is
disabled for PyROOT because it can cause trouble with overload
resolution. If a function has overloads for both ordinary objects and
smart pointers, then the implicit conversion to smart pointers can
result in the smart pointer overload being hit, even though there would
be an overload for the regular object. Since PyROOT didn't have this
feature before 6.32 anyway, disabling it was the safest option.

Closes root-project#15117.
@guitargeek guitargeek self-assigned this Apr 4, 2024
@phsft-bot
Copy link

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

Copy link

github-actions bot commented Apr 4, 2024

Test Results

    12 files      12 suites   2d 1h 1m 54s ⏱️
 2 606 tests  2 606 ✅ 0 💤 0 ❌
29 292 runs  29 292 ✅ 0 💤 0 ❌

Results for commit b4c7f24.

@guitargeek
Copy link
Contributor Author

Needs a backport to 6.32

Copy link
Member

@vepadulano vepadulano 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 the quick fix!

@guitargeek guitargeek merged commit 062bba9 into root-project:master Apr 4, 2024
16 of 17 checks passed
@guitargeek guitargeek deleted the implicit_to_smartptr branch April 4, 2024 15:15
guitargeek added a commit to root-project/roottest that referenced this pull request Apr 4, 2024
@tmadlener
Copy link
Contributor

Needs a backport to 6.32

Has this already happened? We see the same problem that we saw before now on dev4 LCG builds. In case the backport has not happened yet we would know why, otherwise we would have to investigate and see if this is a different issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PyROOT update broke some existing code
4 participants