-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 update broke some existing code #15117
Comments
Closes root-project#15117. This is a direct port of a change that cppyy applied to ROOT meta. See https://github.com/wlav/cppyy-backend/blob/master/cling/src/core/metacling/src/TClingCallFunc.cxx#L468.
Thank you so much for the detailed report! I see, so cppyy upstream made a change to At least the problem is understood! |
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.
Okay, the updates to CallFunc would be too controversial. See also: #14426 Also, I don't like it that the I suggest to disable the implicit promotion to smart pointers now: Then the behavior of PyROOT is the same as in ROOT 6.30 for your reproducer, which I think is safest. What do you think? |
And about the debatable overload choice, I suggest to open an issue in |
Thanks for the quick action on this one. Actually looking into #14425, I think this might be considered a duplicate of that. Sorry, I didn't see that before I opened this one. Regarding the proposed changes in #15125, I am OK with that obviously, since it doesn't require work on our end ;) I am not sure how cppyy handles the lifetimes of objects that are referred to via a regular pointer, but an implicit conversion to a smart pointer seems like a good source of all kinds of issues there. So also from that point of view, I think it is safe to not have that conversion. I will report the overload resolution issue upstream. |
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.
Marked a "Fixed in: not applicable" because this was a regression and not a bug in any released version of ROOT. |
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.
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.
This needs a backport to 6.32, or maybe an additional fix, see #15125 (comment) |
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.
Thank you for the reminder @andresailer! Sorry that this one slipped through the cracks |
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.
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.
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.
Check duplicate issues.
Description
The recent update to PyROOT, broke some existing code on our end. It's not entirely trivial but I have managed to put together a "minimal" reproducer that triggers the issue. I am fairly certain that the new behavior is wrong, but I am not completely certain.
This might also be related to #15085, but I am not entirely sure, as that could also be a c++ standard issue(?). I am also pretty sure that this is an issue in PyROOT and not in cppyy (see at the very bottom).
Reproducer
ROOT version
This works with 6.30.04 (and previous) but starts to fail with the current ROOT master (via
dev3
onubuntu2204
).Installation method
6.30.04 via spack, ROOT master via LCG
dev3
nightly buildOperating system
Ubuntu 22.04
Additional context
With 6.30.04 I get
However, with the updated cppyy I get:
Which is in principle correct, but I don't understand why it chooses that overload in the first place, as it should use the templated function instead.
Curiously, one thing that makes things "work" with the current master branch is to change
However, that changes behavior slightly because now this calls the
unique_ptr
overload instead:Additionally, this change breaks with root 6.30.04 leading to a segmentation because it also tries to call the wrong overload.
stack trace
Finally, I have checked this with "pure"
cppyy
(3.1.2). There I observe the following:void foo(std::unique_ptr<Base>)
(the original), then cppyy calls the templated versionvoid foo(std::unique_ptr<Base>&&)
, then cppyy also calls the unique_ptr versionThe text was updated successfully, but these errors were encountered: