Henry Robinson has posted comments on this change.

Change subject: IMPALA-5174: Add hidden flags to gflags (2.2.0-p1)
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6672/1/source/gflags/gflags-2.2.0-patches/0001-Allow-hidden-flags-e.g.-DEFINE_int32_hidden.patch
File 
source/gflags/gflags-2.2.0-patches/0001-Allow-hidden-flags-e.g.-DEFINE_int32_hidden.patch:

PS1, Line 239: +  namespace fLS {                                               
            \
             : +    using ::fLS::clstring;                                      
            \
             : +    using ::fLS::StringFlagDestructor;                          
            \
             : +    static union { void* align; char s[sizeof(clstring)]; } 
s_##name[2];    \
             : +    clstring* const FLAGS_no##name = ::fLS::                    
            \
             : +                                   
dont_pass0toDEFINE_string(s_##name[0].s, \
             : +                                                             
val);          \
             : +    static GFLAGS_NAMESPACE::FlagRegisterer o_##name(           
            \
             : +        #name, MAYBE_STRIPPED_HELP(txt), __FILE__,              
            \
             : +        FLAGS_no##name, new (s_##name[1].s) 
clstring(*FLAGS_no##name),      \
             : +        true);                                                  
            \
             : +    static StringFlagDestructor d_##name(s_##name[0].s, 
s_##name[1].s);     \
             : +    extern GFLAGS_DLL_DEFINE_FLAG clstring& FLAGS_##name;       
            \
             : +    using fLS::FLAGS_##name;                                    
            \
             : +    clstring& FLAGS_##name = *FLAGS_no##name;                   
            \
             : +  }                                                             
            \
             : +  using fLS::FLAGS_##name
> There's a bunch of code here that will be hard to identify if it starts div
Not sure - I think this code needs to live in a macro, but macros aren't 
recursive so I can't factor out anything. Do you have any ideas? Maybe just a 
strongly worded comment?


-- 
To view, visit http://gerrit.cloudera.org:8080/6672
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I48d718bac3dbf548cdaefc70f8f497bbebe30da6
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-HasComments: Yes

Reply via email to