New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 1382 - Usage of "using std::size_t;" in Eigen/Core
Summary: Usage of "using std::size_t;" in Eigen/Core
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.3 (current stable)
Hardware: All All
: Normal Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-23 13:55 UTC by patrikhuber
Modified: 2017-01-23 21:25 UTC (History)
3 users (show)



Attachments

Description patrikhuber 2017-01-23 13:55:50 UTC
I discovered recently that Eigen has a line

// we use size_t frequently and we'll never remember to prepend it with std:: everytime just to
// ensure QNX/QCC support
using std::size_t;

in its Eigen/Core header. While I do understand the rationale given in the comment, I consider this to be very intrusive and bad practice. A header-only library shouldn't have any using directives in its headers: It will affect everyone's project that will include this header. I actually wondered in my projects multiple times why the compilers didn't complain when I didn't prefix size_t with std::, and I knew that I hadn't used a using directive in my headers - now I know that it was Eigen's header. I would strongly argue to change this.

There's a few other using directives in the header (e.g. I found "using std::ptrdiff_t;"), the same argument would go for these - but size_t is most bothering.
Comment 1 Gael Guennebaud 2017-01-23 15:49:46 UTC
I do agree, though on my computers (OSX,Linux) using either gcc, clang, or icc, the following compile fine:

#include <cstddef>
int main() {
  size_t x = 2;
  return x;
}

so this won't make any difference except on QNX and perhaps some other exotic platforms.

Since we cannot test Eigen of those platforms, we need a way to make sure that writing size_t instead of std::size_t fails to compile. Any idea?
Comment 2 Christoph Hertzberg 2017-01-23 17:45:19 UTC
Could we just move the using std::size_t inside our namespace Eigen { }?
Or would this fail if we are in some sub-namespace of Eigen?
Comment 3 Gael Guennebaud 2017-01-23 20:40:38 UTC
(In reply to Christoph Hertzberg from comment #2)
> Could we just move the using std::size_t inside our namespace Eigen { }?
> Or would this fail if we are in some sub-namespace of Eigen?

yes, this could work.
Comment 4 Gael Guennebaud 2017-01-23 21:25:49 UTC
Here you go:

https://bitbucket.org/eigen/eigen/commits/44f398045cff/ (devel)
https://bitbucket.org/eigen/eigen/commits/b8de228d7a96/ (3.3)
Summary:     Add std:: namespace prefix to all (hopefully) instances if size_t/ptrdfiff_t

https://bitbucket.org/eigen/eigen/commits/f84b7350a9e2/ (devel)
https://bitbucket.org/eigen/eigen/commits/8c963af45ab1/ (3.3)
Summary:     Bug 1382: move using std::size_t/ptrdiff_t to Eigen's namespace (still better than the global namespace!)


If someone as a suggestion to safely get rid of the using namespace statements or has issues with the new approach (using std::size_t/ptrdiff_t within Eigen namespace), feel free to re-open.

Note You need to log in before you can comment on or make changes to this bug.