On 08/23/2013 02:18 PM, Paul Berry wrote:

The disadvantages are that (a) we need an explicit enum value for 0,
and (b) we can't use related operators like |= unless we define
additional overloads.

Disadvantage (a) is trivial, not really a problem.

Disadvantage (b) can be made painless with the macro I discuss below.


So what do folks think?  Is it worth it?


Yes, I think it's worth it. The code becomes more readable and
more type safe, as evidenced by the diff lines like this:
> -                       unsigned flags,
> +                       enum brw_urb_write_flags flags,

If we continue to do this to other enums, then we should probably
define a convenience macro to define all the needed overloaded
bit operators. Like this:

  #define BRW_CXX_ENUM_OPS(type_name) \
      static inline type_name \
      operator|(type_name x, type_name y) \
      { \
         return (type_name) ((int) x | (int) y); \
      } \
      \
      static inline type_name \
      operator&(........) \
      ........ and more operators


+/**
+ * Allow brw_urb_write_flags enums to be ORed together (normally C++ wouldn't
+ * allow this without a type cast).
+ */
+inline enum brw_urb_write_flags
+operator|(enum brw_urb_write_flags x, enum brw_urb_write_flags y)
+{
+   return (enum brw_urb_write_flags) ((int) x | (int) y);
+}
+#endif

I think the comment is distracting rather than helpful. There is no need for C++
code to apologize for being C++ code.

For what it's worth, this patch is
Reviewed-by: Chad Versace <chad.vers...@linux.intel.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to