|Summary:||-Wshadow and -Wsign-conversion warnings|
|Product:||Eigen||Reporter:||David Evans <davidje13>|
|Component:||Core - general||Assignee:||Nobody <eigen.nobody>|
|Severity:||Unknown||CC:||chtz, gael.guennebaud, jacob.benoit.1, rmann|
|Version:||3.3 (current stable)|
|Bug Depends on:|
Description David Evans 2013-08-07 13:32:35 UTC
Comment 1 Rick Mann 2014-03-18 23:24:26 UTC
I'd like to try to up the importance of fixing this bug. Recently in our own code we ran into a bug due to an improperly shadowed variable, so we turned on the warning. This resulted in hundreds of warnings from Eigen (mostly duplicates). Now, it's possible to suppress these warnings by using -isystem, but it really would be better for Eigen to not shadow variables at all. It's easy to introduce errors when doing so, and one should always strive to be able to build with all warnings turned on and no warning messages emitted. Xcode doesn't directly support -isystem, and as a result, CMake doesn't know how to add -isystem search paths when generating Xcode projects. I realize these are limitations of Xcode and CMake (especially since one CAN set Other C Flags in Xcode to include -isystem). But the initial assertion stands; good defensive coding precludes allowing shadowing. Examples: eigen/Eigen/src/Core/products/TriangularSolverMatrix.h:136:23: warning: declaration shadows a local variable [-Wshadow] Index s = IsLower ? i+1 : i-rs; ^ eigen/Eigen/src/Core/products/TriangularSolverMatrix.h:118:19: note: previous declaration is here Index s = IsLower ? k2+k1 : i+1; eigen/Eigen/src/SVD/JacobiSVD.h:395:16: warning: declaration shadows a local variable [-Wshadow] Scalar z = abs(work_matrix.coeff(p,q)) / work_matrix.coeff(p,q); ^ eigen/Eigen/src/SVD/JacobiSVD.h:375:12: note: previous declaration is here Scalar z; ^
Comment 2 Christoph Hertzberg 2014-03-19 01:15:52 UTC
Gael started fixing many of these. It's quite likely that 3.3 will be (mostly) shadow free.
Comment 3 Rick Mann 2014-03-19 01:17:51 UTC
That's great news! You guys have a much better attitude than Boost. They refuse to fix their shadows.
Comment 4 Gael Guennebaud 2015-06-09 09:36:18 UTC
Fixed for clang: https://bitbucket.org/eigen/eigen/commits/3c6892351266/ -Wshadow is still bogus in gcc, so I'll wait their answer before making our code unnecessarily uglier: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57709
Comment 5 Gael Guennebaud 2015-06-09 11:26:36 UTC
(In reply to Gael Guennebaud from comment #4) > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57709 followup: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66472
Comment 6 Christoph Hertzberg 2015-08-19 19:40:10 UTC
The dev-branch should be essentially shadow-free. I don't think this is worth back-porting to 3.2
Comment 7 Rick Mann 2015-08-19 20:05:13 UTC
Thank you. I don't think it needs to be backported, either. We can just as easily update to 3.3.
Comment 8 Nobody 2019-12-04 12:32:25 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/640.