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

Match Pythran interface when generating index operator #1946

Conversation

serge-sans-paille
Copy link
Contributor

Otherwise calls to a[i,j] are turned into a[(i,j)] in C++, , being the sequence operator.

@scoder
Copy link
Contributor

scoder commented Oct 23, 2017

Hmm, that can't really be all. There's probably a second part of this change missing in ExprNodes.py, around line 4197 and/or 4228, which call pythran_indexing_code() to generate the actual index access.

@scoder
Copy link
Contributor

scoder commented Oct 24, 2017

Yeah, I tried that, too (and that specific change is actually best applied inside of pythran_indexing_code()), but it gives me invalid C++ code for the numpy_pythran test:

numpy_pythran.cpp: In function ‘PyObject* __pyx_pf_13numpy_pythran_6_diffuse_numpy(PyObject*, pythonic::types::ndarray<double, 2ul>, int)’:
numpy_pythran.cpp:3992:173: error: ‘::type’ has not been declared
   typename std::remove_reference<decltype(std::declval<pythonic::types::ndarray<double,2>>()(pythonic::types::contiguous_slice(0,0),pythonic::types::contiguous_slice(0,0))>::type __pyx_t_10;

The previous declaration (in current Cython master) was:

   typename std::remove_reference<decltype(std::declval<pythonic::types::ndarray<double,2>>()[pythonic::types::contiguous_slice(0,0),pythonic::types::contiguous_slice(0,0)])>::type __pyx_t_10;

You can directly run the pythran test like this:

CFLAGS="-Wall -O0 -ggdb" python runtests.py -vv --backend=cpp --debug pythran

I do not know how the C++ code is supposed to look like here, so I'm not in a position to fix it.

@@ -76,7 +76,8 @@ def index_code(idx):
raise ValueError("unsupported indexing type %s!" % idx.type)

indexing = ",".join(index_code(idx) for idx in indices)
return type_remove_ref("decltype(std::declval<%s>()[%s])" % (pythran_type(type_), indexing))
index = '[%s]' if len(indices) == 1 else '(%s)'
return type_remove_ref(("decltype(std::declval<%s>()" + index) % (pythran_type(type_), indexing))
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, there was a closing parenthesis missing here, after the index. With that, it's now compiling and giving the correct result for me.

@scoder scoder closed this in 150c428 Oct 24, 2017
@serge-sans-paille
Copy link
Contributor Author

serge-sans-paille commented Oct 24, 2017 via email

@scoder scoder added this to the 0.27.3 milestone Oct 24, 2017
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.

None yet

2 participants