dropbox / djinni

A tool for generating cross-language type declarations and interface bindings.
Apache License 2.0
2.88k stars 488 forks source link

Problems with constexpr in G++ #409

Open paulocoutinhox opened 5 years ago

paulocoutinhox commented 5 years ago

Hi,

Im trying use the flag resource of djinni in both G++ and CLANG++. On clang no problem, but on G++ i got error when use std=c++11 but not when use std=c++14 or std=c++17. This is the source:

#include <functional>

enum class LoggerLevel : unsigned {
    NO_LEVEL = 0,
    VERBOSE = 1 << 0,
    DEBUG = 1 << 1,
    INFO = 1 << 2,
    WARNING = 1 << 3,
    ERROR = 1 << 4,
    FATAL = 1 << 5,
    ALL_LEVELS = 0 | VERBOSE | DEBUG | INFO | WARNING | ERROR | FATAL,
};

constexpr LoggerLevel operator|(LoggerLevel lhs, LoggerLevel rhs) noexcept {
    return static_cast<LoggerLevel>(static_cast<unsigned>(lhs) | static_cast<unsigned>(rhs));
}

constexpr LoggerLevel& operator|=(LoggerLevel& lhs, LoggerLevel rhs) noexcept {
    return (lhs = lhs | rhs);
}

int main()
{
    auto x = LoggerLevel::ALL_LEVELS;
    return 0;
}

This is the error:

<source>: In function 'constexpr LoggerLevel& operator|=(LoggerLevel&, LoggerLevel)':

<source>:19:17: error: expression '(lhs = operator|(lhs, rhs))' is not a constant expression

     return (lhs = lhs | rhs);

            ~~~~~^~~~~~~~~~~~

Compiler returned: 1

https://godbolt.org/z/R9oVpF

On godbolt change STD to 11 or to 14 to see the problem.

paulocoutinhox commented 5 years ago

The code incompatible with C++11 in constexpr specs:

https://stackoverflow.com/a/52706427/1001566

artwyman commented 5 years ago

Looks like a legit bug, based on the analysis on StackOverflow. Djinni is intended to work with C++11, but it looks like the flags feature isn't fully compatible with that language version.

Note that the flags feature wasn't developed by, and isn't used by folks at Dropbox currently. It was developed by @mknejp and landed after some fixes by @choiip so they might be good contacts to take a look at fixing this.

mknejp commented 5 years ago

Unfortunately I don't think there's a solution here for C++11 that doesn't involve removing those operators. This was a huge oversight on my part.