Move Eigen::all,last,lastp1,lastN to Eigen::placeholders::.

These names are so common, IMO they should not exist directly in the
`Eigen::` namespace.  This prevents us from using the `last` or `all`
names for any parameters or local variables, otherwise spitting out
warnings about shadowing or hiding the global values.  Many external
projects (and our own examples) also heavily use
```
using namespace Eigen;
```
which means these conflict with external libraries as well, e.g.
`std::fill(first,last,value)`.

It seems originally these were placed in a separate namespace
`Eigen::placeholders`, which has since been deprecated.  I propose
to un-deprecate this, and restore the original locations.

These symbols are also imported into `Eigen::indexing`, which
additionally imports `fix` and `seq`. An alternative is to remove the
`placeholders` namespace and stick with `indexing`.

NOTE: this is an API-breaking change.

Fixes #2321.
This commit is contained in:
Antonio Sanchez 2021-09-15 14:10:29 -07:00
parent 44da7a3b9d
commit 5dac0b53c9
10 changed files with 83 additions and 57 deletions

View File

@ -74,7 +74,7 @@ template<typename T> struct cleanup_seq_incr {
typedef typename cleanup_index_type<T,DynamicIndex>::type type;
};
}
} // namespace internal
//--------------------------------------------------------------------------------
// seq(first,last,incr) and seqN(first,size,incr)
@ -321,6 +321,7 @@ seq(const symbolic::BaseExpr<FirstTypeDerived> &f, const symbolic::BaseExpr<Last
#endif // EIGEN_PARSED_BY_DOXYGEN
namespace placeholders {
#if EIGEN_HAS_CXX11 || defined(EIGEN_PARSED_BY_DOXYGEN)
/** \cpp11
@ -331,9 +332,9 @@ seq(const symbolic::BaseExpr<FirstTypeDerived> &f, const symbolic::BaseExpr<Last
* \sa lastN(SizeType), seqN(FirstType,SizeType), seq(FirstType,LastType,IncrType) */
template<typename SizeType,typename IncrType>
auto lastN(SizeType size, IncrType incr)
-> decltype(seqN(Eigen::last-(size-fix<1>())*incr, size, incr))
-> decltype(seqN(Eigen::placeholders::last-(size-fix<1>())*incr, size, incr))
{
return seqN(Eigen::last-(size-fix<1>())*incr, size, incr);
return seqN(Eigen::placeholders::last-(size-fix<1>())*incr, size, incr);
}
/** \cpp11
@ -344,12 +345,14 @@ auto lastN(SizeType size, IncrType incr)
* \sa lastN(SizeType,IncrType, seqN(FirstType,SizeType), seq(FirstType,LastType) */
template<typename SizeType>
auto lastN(SizeType size)
-> decltype(seqN(Eigen::last+fix<1>()-size, size))
-> decltype(seqN(Eigen::placeholders::last+fix<1>()-size, size))
{
return seqN(Eigen::last+fix<1>()-size, size);
return seqN(Eigen::placeholders::last+fix<1>()-size, size);
}
#endif
} // namespace placeholders
namespace internal {
// Convert a symbolic span into a usable one (i.e., remove last/end "keywords")
@ -389,25 +392,25 @@ struct get_compile_time_incr<ArithmeticSequence<FirstType,SizeType,IncrType> > {
* \code using namespace Eigen::indexing; \endcode
* is equivalent to:
* \code
using Eigen::all;
using Eigen::fix;
using Eigen::seq;
using Eigen::seqN;
using Eigen::lastN; // c++11 only
using Eigen::last;
using Eigen::lastp1;
using Eigen::fix;
using Eigen::placeholders::all;
using Eigen::placeholders::last;
using Eigen::placeholders::lastN; // c++11 only
using Eigen::placeholders::lastp1;
\endcode
*/
namespace indexing {
using Eigen::all;
using Eigen::fix;
using Eigen::seq;
using Eigen::seqN;
using Eigen::placeholders::all;
using Eigen::placeholders::last;
#if EIGEN_HAS_CXX11
using Eigen::lastN;
using Eigen::placeholders::lastN;
#endif
using Eigen::last;
using Eigen::lastp1;
using Eigen::fix;
using Eigen::placeholders::lastp1;
}
} // end namespace Eigen

View File

@ -98,7 +98,7 @@ class IndexedViewImpl;
* - decltype(ArrayXi::LinSpaced(...))
* - Any view/expressions of the previous types
* - Eigen::ArithmeticSequence
* - Eigen::internal::AllRange (helper for Eigen::all)
* - Eigen::internal::AllRange (helper for Eigen::placeholders::all)
* - Eigen::internal::SingleRange (helper for single index)
* - etc.
*

View File

@ -17,7 +17,11 @@ namespace Eigen {
namespace internal {
struct symbolic_last_tag {};
}
} // namespace internal
namespace placeholders {
typedef symbolic::SymbolExpr<internal::symbolic_last_tag> last_t;
/** \var last
* \ingroup Core_Module
@ -30,34 +34,16 @@ struct symbolic_last_tag {};
* A typical usage example would be:
* \code
* using namespace Eigen;
* using Eigen::last;
* using Eigen::placeholders::last;
* VectorXd v(n);
* v(seq(2,last-2)).setOnes();
* \endcode
*
* \sa end
*/
static const symbolic::SymbolExpr<internal::symbolic_last_tag> last; // PLEASE use Eigen::last instead of Eigen::placeholders::last
static const last_t last;
/** \var lastp1
* \ingroup Core_Module
*
* Can be used as a parameter to Eigen::seq and Eigen::seqN functions to symbolically
* reference the last+1 element/row/columns of the underlying vector or matrix once
* passed to DenseBase::operator()(const RowIndices&, const ColIndices&).
*
* This symbolic placeholder supports standard arithmetic operations.
* It is essentially an alias to last+fix<1>.
*
* \sa last
*/
#ifdef EIGEN_PARSED_BY_DOXYGEN
static const auto lastp1 = last+fix<1>;
#else
// Using a FixedExpr<1> expression is important here to make sure the compiler
// can fully optimize the computation starting indices with zero overhead.
static const symbolic::AddExpr<symbolic::SymbolExpr<internal::symbolic_last_tag>,symbolic::ValueExpr<Eigen::internal::FixedInt<1> > > lastp1(last+fix<1>());
#endif
} // namespace placeholders
namespace internal {
@ -70,7 +56,7 @@ FixedInt<N> eval_expr_given_size(FixedInt<N> x, Index /*size*/) { return x; }
template<typename Derived>
Index eval_expr_given_size(const symbolic::BaseExpr<Derived> &x, Index size)
{
return x.derived().eval(last=size-1);
return x.derived().eval(Eigen::placeholders::last=size-1);
}
// Extract increment/step at compile time
@ -165,23 +151,44 @@ template<int Size> struct get_compile_time_incr<AllRange<Size> > {
} // end namespace internal
namespace placeholders {
typedef symbolic::AddExpr<symbolic::SymbolExpr<internal::symbolic_last_tag>,symbolic::ValueExpr<Eigen::internal::FixedInt<1> > > lastp1_t;
typedef Eigen::internal::all_t all_t;
/** \var lastp1
* \ingroup Core_Module
*
* Can be used as a parameter to Eigen::seq and Eigen::seqN functions to symbolically
* reference the last+1 element/row/columns of the underlying vector or matrix once
* passed to DenseBase::operator()(const RowIndices&, const ColIndices&).
*
* This symbolic placeholder supports standard arithmetic operations.
* It is essentially an alias to last+fix<1>.
*
* \sa last
*/
#ifdef EIGEN_PARSED_BY_DOXYGEN
static const auto lastp1 = last+fix<1>;
#else
// Using a FixedExpr<1> expression is important here to make sure the compiler
// can fully optimize the computation starting indices with zero overhead.
static const lastp1_t lastp1(last+fix<1>());
#endif
/** \var end
* \ingroup Core_Module
* \sa lastp1
*/
static const lastp1_t end = lastp1;
/** \var all
* \ingroup Core_Module
* Can be used as a parameter to DenseBase::operator()(const RowIndices&, const ColIndices&) to index all rows or columns
*/
static const Eigen::internal::all_t all; // PLEASE use Eigen::all instead of Eigen::placeholders::all
static const Eigen::internal::all_t all;
namespace placeholders {
typedef symbolic::SymbolExpr<internal::symbolic_last_tag> last_t;
typedef symbolic::AddExpr<symbolic::SymbolExpr<internal::symbolic_last_tag>,symbolic::ValueExpr<Eigen::internal::FixedInt<1> > > end_t;
typedef Eigen::internal::all_t all_t;
EIGEN_DEPRECATED static const all_t all = Eigen::all; // PLEASE use Eigen::all instead of Eigen::placeholders::all
EIGEN_DEPRECATED static const last_t last = Eigen::last; // PLEASE use Eigen::last instead of Eigen::placeholders::last
EIGEN_DEPRECATED static const end_t end = Eigen::lastp1; // PLEASE use Eigen::lastp1 instead of Eigen::placeholders::end
}
} // namespace placeholders
} // end namespace Eigen

View File

@ -37,7 +37,9 @@ namespace Eigen {
* std::cout << expr98.eval(x=6) << "\n";
* \endcode
*
* It is currently only used internally to define and manipulate the Eigen::last and Eigen::lastp1 symbols in Eigen::seq and Eigen::seqN.
* It is currently only used internally to define and manipulate the
* Eigen::placeholders::last and Eigen::placeholders::lastp1 symbols in
* Eigen::seq and Eigen::seqN.
*
*/
namespace symbolic {

View File

@ -218,7 +218,7 @@ operator()(const IndicesT (&indices)[IndicesN]) EIGEN_INDEXED_VIEW_METHOD_CONST
*
* Each parameter must either be:
* - An integer indexing a single row or column
* - Eigen::all indexing the full set of respective rows or columns in increasing order
* - Eigen::placeholders::all indexing the full set of respective rows or columns in increasing order
* - An ArithmeticSequence as returned by the Eigen::seq and Eigen::seqN functions
* - Any %Eigen's vector/array of integers or expressions
* - Plain C arrays: \c int[N]
@ -235,7 +235,7 @@ operator()(const IndicesT (&indices)[IndicesN]) EIGEN_INDEXED_VIEW_METHOD_CONST
* method will returns a Block object after extraction of the relevant information from the passed arguments. This is the case
* when all arguments are either:
* - An integer
* - Eigen::all
* - Eigen::placeholders::all
* - An ArithmeticSequence with compile-time increment strictly equal to 1, as returned by Eigen::seq(a,b), and Eigen::seqN(a,N).
*
* Otherwise a more general IndexedView<Derived,RowIndices',ColIndices'> object will be returned, after conversion of the inputs

View File

@ -15,7 +15,7 @@ All the aforementioned operations are handled through the generic DenseBase::ope
Each argument can be:
- An integer indexing a single row or column, including symbolic indices.
- The symbol Eigen::all representing the whole set of respective rows or columns in increasing order.
- An ArithmeticSequence as constructed by the Eigen::seq, Eigen::seqN, or Eigen::lastN functions.
- An ArithmeticSequence as constructed by the Eigen::seq, Eigen::seqN, or Eigen::placeholders::lastN functions.
- Any 1D vector/array of integers including %Eigen's vector/array, expressions, std::vector, std::array, as well as plain C arrays: `int[N]`.
More generally, it can accepts any object exposing the following two member functions:
@ -114,7 +114,8 @@ Here are some examples for a 2D array/matrix \c A and a 1D array/vector \c v.
As seen in the last exemple, referencing the <i> last n </i> elements (or rows/columns) is a bit cumbersome to write.
This becomes even more tricky and error prone with a non-default increment.
Here comes \link Eigen::lastN(SizeType) Eigen::lastN(size) \endlink, and \link Eigen::lastN(SizeType,IncrType) Eigen::lastN(size,incr) \endlink:
Here comes \link Eigen::placeholders::lastN(SizeType) Eigen::placeholders::lastN(size) \endlink, and
\link Eigen::placeholders::lastN(SizeType,IncrType) Eigen::placeholders::lastN(size,incr) \endlink:
<table class="manual">
<tr>

View File

@ -36,7 +36,12 @@
#include <vector>
#include "main.h"
using Eigen::placeholders::all;
using Eigen::placeholders::last;
using Eigen::placeholders::lastp1;
#if EIGEN_HAS_CXX11
using Eigen::placeholders::lastN;
#include <array>
#endif

View File

@ -10,6 +10,9 @@
#include "main.h"
using Eigen::placeholders::last;
using Eigen::placeholders::all;
template<typename T1,typename T2>
typename internal::enable_if<internal::is_same<T1,T2>::value,bool>::type
is_same_eq(const T1& a, const T2& b)

View File

@ -133,6 +133,7 @@ void test_stl_iterators(int rows=Rows, int cols=Cols)
ColMatrixType A = ColMatrixType::Random(rows,cols);
const ColMatrixType& cA(A);
RowMatrixType B = RowMatrixType::Random(rows,cols);
using Eigen::placeholders::last;
Index i, j;

View File

@ -19,6 +19,10 @@
#include "main.h"
using Eigen::placeholders::last;
using Eigen::placeholders::lastp1;
using Eigen::placeholders::all;
template<typename T1,typename T2>
bool is_same_symb(const T1& a, const T2& b, Index size)
{