impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Apple <jbap...@cloudera.com>
Subject Re: IMPALA-3738 advice requested
Date Tue, 27 Jun 2017 21:06:33 GMT
I'm not sure it's worth the effort to write all of the stringify methods.
My order of preference is:

1. Just use friend with the current conversion-to-int behavior.

2. Do something from this list so there won't be manual work for each new
enum decl:
https://stackoverflow.com/questions/28828957/enum-to-string-in-modern-c-and-future-c17-c20

3. Write a stringify function for each enum type.

On Tue, Jun 27, 2017 at 1:42 PM, Steve Carlin <stevec@arcadiadata.com>
wrote:

> Hi all,
>
> Short time user, first time poster.  This post is in reference to the
> following Jira:  https://issues.apache.org/jira/browse/IMPALA-3738
>
> I'm currently looking at IMPALA-3738 which involves changing enums to c+11
> style (e.g. "enum A" to "enum class A"
>
> Unfortunately, as with everything in life, there are complications.
> Nothing major, but wanted some advice before I changed a lot of code only
> to find out it gets rejected.
>
> Chrome did a similar fix and had a similar complication.  For details, you
> can reference this code review:
> https://codereview.chromium.org/835783003/patch/1/10003
>
> But the problem is this:  Simply changing it to c+11 style gives us a
> compiler error with the macro DCHECK_EQ.  The solution which they used in
> chrome was to add a "friend" class.  I'm ok with doing this unless someone
> has another suggestion.
>
> On top of this though, I have another question.  The DCHECK_EQ macro will
> print a message such as this:  Check failed: x == y (1 vs. 3).
> Specifically, my question has to do with the "1 vs 3" portion.  Do we want
> to print a more user friendly message?  If so, we would have to do
> something similar to what the chrome checkin has:  They call a method that
> converts the enum to a string and prints out the string value (they already
> had these methods in place before their change).  Is it worth doing this
> kind of rewrite, or are we ok with just having the enum value?
>
> Thanks!
>
> Steve
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message