This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
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: 2019-12-04 16:45 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.
Comment 5 Nobody 2019-12-04 16:45:17 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/1382.

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