@FilipeMaia Can you remove the "Closes #894" from the commit message and put it as a description in the PR?

Can i just force push the amended commit message or do i need a new pull request?

Hi @FilipeMaia, Thanks for finding this issue.

The problem is that af::index is only used to simplify the operator() overloads. It allows us to support any arbitrary combination of int, af::seq or af::array objects to index into an array. It is never meant to be created. I think a better solution would be to make the af::index default constructor private so that it cannot be created without referencing one of the other constructors.

Force push is fine.

@umar456, I think that having things in the public API which are not supposed to be manipulated by the users, such as af::index and af::array::array_proxy is a sure way to lead to problems in the future.

Just making the default constructor private is not enough. You also need to avoid the creation of af::index::operator=, and there might be other things we haven't thought about.

@umar456 For better or worse it is out in the public. We can't change the API now.

@FilipeMaia Can you make the code from the failure case as a test ? You can make it a new test in test/index.cpp.

I'm not sure if I succeeded making a test. I'm don't know how to test them...

That should be enough 👍 Any code snippet that was failing makes a valid test.

@FilipeMaia Yeah, I forgot about the rule of 3/5. Could you add operator=(index&&) and index(index&&) for the index class with c++11 guards (see:https://github.com/arrayfire/arrayfire/blob/devel/include/af/array.h#L51). These functions will not need to increment the reference count for the array object.

I still think we should make the default constructor private. The index class was not designed to be used this way. @pavanky This change will not break ABI but might cause issues when recompiling for those who used af::index incorrectly.

@umar456 such a change would break the ABI because the copy constructor, which is currently public, would become private.

ABI would not break because the Linker will still create the symbol. The API on the other hand will be different. This will cause issues during recompilation of code which makes use of the af::index default constructor. I would like to make this change but I understand it can cause issues with some users.

Ok, I didn't realise the private/public part would not impact the symbol.

ok to test

build arrayfire ci