New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 1207 - -Wlogical-op Warning Due to Erroneous Use of `&&` in Integer-Valued `enum`
Summary: -Wlogical-op Warning Due to Erroneous Use of `&&` in Integer-Valued `enum`
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.2
Hardware: All All
: Normal Compilation Problem
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-18 06:15 UTC by ian@geometrian.com
Modified: 2016-05-11 17:57 UTC (History)
3 users (show)



Attachments

Description ian@geometrian.com 2016-04-18 06:15:59 UTC
In "/src/Core/Assign.h" (tested Eigen 3.2.8), there are a few `enum`s that use `&&`.  E.g.

  enum {
    //...
    MightVectorize = StorageOrdersAgree
                  && (int(Derived::Flags) & int(OtherDerived::Flags) & ActualPacketAccessBit),
    //...
  };

The problem is that (for the above example) `StorageOrdersAgree` isn't a boolean; it's another element of the enum.  This produces GCC warnings like:

/usr/include/eigen3/Eigen/src/Core/Assign.h:49:19: warning: logical ‘and’ applied to non-boolean constant [-Wlogical-op]
                   && (int(Derived::Flags) & int(OtherDerived::Flags) & ActualPacketAccessBit),
                   ^

There are several other examples in that file.
Comment 1 Christoph Hertzberg 2016-04-18 09:45:05 UTC
Hm, we probably should fix that ...
Essentially, we have two alternatives: Casting every enum to bool/int before using it (we already cast a lot to int), or declaring enums as static const bool/int instead. Either way will require touching most of our internal logic.

We had a discussion about the latter some time ago: There is a minor drawback that this won't allow to use enums directly in functions which expect (const Index&) parameters:
http://thread.gmane.org/gmane.comp.lib.eigen/4960

So casting everything to bool() is at least easier/safer, but slightly reduces readability/usability (e.g., user which want to use our flags need to cast them to bool as well, if they want to comply with -Wlogical-op).
Comment 2 Gael Guennebaud 2016-04-25 14:43:48 UTC
I'd go with the casting solution, which is already what we are doing.
Comment 3 Christoph Hertzberg 2016-05-11 17:57:17 UTC
Fixed here (devel branch only):
https://bitbucket.org/eigen/eigen/commits/5cdfa660882

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