2015-03-15 9:09 GMT+01:00 Tim Shen <timshe...@gmail.com>: > Did a little bit refectoring on regex_constants, so that users won't > be polluted too much (they are not enum types anymore).
Your implementation choice is an interesting approach. I believe that a strict reading of the library specification does not allow that form, because you are using as bitmask type a class type that is not std::bitset: [bitmask.types]: Each bitmask type can be implemented as an enumerated type that overloads certain operators, as an integer type, or as a bitset (20.6). One could argue that this restriction to this specific set of types could be considered as a Standard Library defect, but no-one has yet submitted one ;-), so it depends on the decision of the maintainers here whether it would be better to follow that strict reading of the Standard or not. What I would consider as more controversial are the following things: a) The class takes advantage of a deprecated rule to implicitly declare a copy-assignment operator (because it has a user-declared copy-constructor). I suggest to fix that and add a defaulted one before at some future point this bitmask type is no longer copy-assignable. b) Basically no operation is marked as noexcept. This is user-observable and seems like an unfortunate outcome. c) The presence of a non-explicit conversion to bool allows users to unintentionally write code like std::regex_constants::syntax_option_type a = ..; [..] int x = a + 3; // Oops The presence of this function also *seemingly* allows to implicitly convert syntax_option_type to any integer type: int x = a; // Oops I suggest to make this conversion function explicit. d) One could consider to mark the mutable operations (syntax_option_type& operator&=(syntax_option_type)) as constexpr in C++14 mode, but that is just an idea, not a required functionality. - Daniel
