New user self-registration is disabled due to spam. Please email eigen-core-team @ if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
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`
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.2
Hardware: All All
: Normal Compilation Problem
Assignee: Nobody
Depends on:
Reported: 2016-04-18 06:15 UTC by
Modified: 2016-05-11 17:57 UTC (History)
3 users (show)


Description 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:

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):

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