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

Support for references in for loop? #3063

Open
gerion0 opened this issue Jul 31, 2019 · 6 comments

Comments

@gerion0
Copy link

commented Jul 31, 2019

Given an input:

for entry in my_cpp_function():
    entry.foo()

Seems to generate a C++ code like (assuming my_cpp_function return a vector[Foo]):

Foo entry;
std::vector<Foo>* vec;

auto it = vec->begin();
while(it != vec->end()) {
     entry = *it;
     it++;
}

In general, this works but makes a copy of entry and assumes that Foo is default constructable.
Is there a way to get Cython to creating entry as reference to Foo?

I have tried:

for entry& in my_cpp_function():
    entry.foo()

but this is invalid syntax.

Here is a complete minimal example:

test.h

#include <vector>
#include <iostream>

struct Foo {
        int a;

        Foo(int a) : a(a) {}
        void foo() { std::cout << "Foo " << a << std::endl; }
};

std::vector<Foo> get_foo_vec() {
        return { Foo(1), Foo(2) };
}

void dummy() {
        for (Foo& bla : get_foo_vec()) {
                bla.foo();
        }
}

test.pxd

from libcpp.vector cimport vector

cdef extern from "test.h":
    cdef cppclass Foo:
        void foo()
    vector[Foo] get_foo_vec()

test.pyx

cimport test

def iterate():
    for entry in get_foo_vec():
        entry.foo()

Compilation with:

cython test.pyx -o test_pyx.cpp -Wextra --cplus -3
g++ -I/usr/include/python3.7m test_pyx.cpp

Error:

test_pyx.cpp: In function 'PyObject* __pyx_pf_4test_iterate(PyObject*)':
test_pyx.cpp:1069:7: error: no matching function for call to 'Foo::Foo()'
   Foo __pyx_v_entry;
@ozars

This comment has been minimized.

Copy link

commented Aug 1, 2019

There is a little mistake in test.pyx. You need to either change import to from test cimport get_foo_vec or use test.get_foo_vec. Your point is valid regardless of that though.

I had similar issues, yet I couldn't find a convenient way to use references from cython. Closest workaround I could come up with was replacing for .. in with while.

With pointer:

# distutils: language = c++                                                                                                                                                                                        
                                                                                                                                                                                                                   
from cython.operator cimport dereference as deref, preincrement as preinc                                                                                                                                          
cimport test                                                                                                                                                                                                       
                                                                                                                                                                                                                   
cdef vector[test.Foo] vec = test.get_foo_vec()
cdef vector[test.Foo].iterator it = vec.begin()
cdef Foo* elm
while it != vec.end():
    elm = &deref(it)  # elm = &*it
    elm.foo()         # elm->foo()
    preinc(it)        # ++it

With a helper function:

cdef inline void process_foo(Foo& obj):
    obj.foo()

while it != vec.end():
    process_foo(deref(it))
    preinc(it)

Of course, neither solves underlying issues, the hard requirement for default constructor and/or declaring variables at the top of the function. Both examples are just workarounds for these restrictions. I agree it would be wonderful if default constructor requirement were to be removed. Ideally, this could be achieved by declaring variables when they are first used. AFAICS, cython has been designed with C language conventions in mind, so it may not be trivial to remove these restrictions without serious refactorization specifically for producing C++.

@robertwb

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

One of the difficulties with references and non-default constructors is the scoping. For example

if condition:
    x = some_value()
else:
    x = other_value()

use(x)

is perfectly valid in Python, but becomes invalid if x is "declared when it is first used." I agree, however, that we could try to do something better here.

@ozars

This comment has been minimized.

Copy link

commented Aug 3, 2019

What I actually meant by "declared when first used" was explicit cdef by user, not cython implicitly inferring where it should be defined. So what I had in my mind would be rather similar to this:

if condition:
    cdef int& x = some_cpp_func()
    ...
else:
    cdef double& x = some_other_cpp_func()
    ...
# x is not declared in this scope.

cdef isn't allowed in such scopes in current implementation (perhaps to be compatible with ANSI C?).

@ozars

This comment has been minimized.

Copy link

commented Aug 4, 2019

I wonder how much work would be required to extend constructors in brace-initialization style, if it is possible of course. Example:

cppclass TestClass:
    TestClass(int, double)
cdef TestClass obj {1, 2.0}

Using brace initialization style over parentheses may be easier to implement (since it gives clear distinction between a function declaration/call and constructor) and it wouldn't change existing semantics of cdef functions. Is this something realistic to implement?

This doesn't address cdef'ing in if, while, for etc. scopes, but implementing custom constructors might be a good step for reaching for that later.

@robertwb

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

Such a change would be a departure from the way scoping works in Python. (It would also require some non-trivial changes to how things are tracked in the compiler as well.)

@scoder

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

I agree that being able to use references in looks seems attractive. I also agree with @robertwb that whatever we do in Cython to achieve this should adhere to Python semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.