Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

cppyy does not retrieve correct size of type when pythonizing vector #11596

Closed
vepadulano opened this issue Oct 18, 2022 · 6 comments
Closed

Comments

@vepadulano
Copy link
Member

root [0] auto a = std::vector<const char*>::value_type();
root [1] a
(const char *) nullptr

Whereas it should return the correct const char* type https://godbolt.org/z/a3s95cex3

Setup

ROOT master and 6.26
Fedora 36
GCC 12

Feel free to change assignees if needed

@vepadulano
Copy link
Member Author

This was the original cause for #11581

@hahnjo
Copy link
Member

hahnjo commented Oct 18, 2022

I may be missing the point here, but to me it seems Cling is doing exactly what you ask it to do: value_type in std::vector is a typedef to the templated type, in this case const char *. Thus, "constructing" a std::vector<const char*>::value_type() yields the default value, a (correctly typed) nullptr. AFAICT you get the equivalent result if you ask for std::vector<int>::value_type(), which is (int) 0.

@vepadulano
Copy link
Member Author

Indeed, it's a bit more subtle. There is a pythonization happening here, which transforms the type given by ::value_type to a string and sets the corresponding Python attribute. Probably for const char * the type is not properly parsed thus the attribute is not set.

@vepadulano
Copy link
Member Author

The issue seems to be with this line in particular:

const std::string& vtype = Cppyy::ResolveName(name+"::value_type");
size_t typesz = Cppyy::SizeOf(vtype);

In the reproducer case, this is equivalent to calling Cppyy::SizeOf("const char *"). This will call SizeOf(const std::string &). In turn, this first calls gROOT->GetType("const char *"), which fails, then falls back to calling SizeOf(GetScope(type_name)).

In GetScope, we reach this part that calls TClass::GetClass. As we can see, this returns nullptr:

root [0] auto cl = TClass::GetClass("const char *", true, true);
root [1] cl
(TClass *) nullptr

Thus, the final result of the initial call to Cppyy::SizeOf(vtype) is 0. This means that cppyy thinks that the size of these types (including const char *, const int * and so on) is zero, thus the attribute is never pythonized.

In upstream cppyy, this is fixed by calling SizeOf with the original type, instead of the resolved one: https://github.com/wlav/CPyCppyy/blob/master/src/Pythonize.cxx#L1789-L1802

So this should be done in our cppyy too. Reassigning the issue to me, it's not related to cling but cppyy itself

@vepadulano vepadulano changed the title std::vector<const char*>::value_type is nullptr cppyy does not retrieve correct size of type when pythonizing vector Oct 18, 2022
@guitargeek
Copy link
Contributor

Since this is fixed upstream, this will the fixed by the cppyy sync PR:
#14507

@guitargeek guitargeek added this to the 6.32/00 milestone Mar 19, 2024
Copy link

Hi @guitargeek, @vepadulano,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@guitargeek guitargeek removed this from the 6.32/00 milestone Mar 20, 2024
@guitargeek guitargeek added this to Issues in Fixed in 6.32.00 via automation Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

5 participants