On Tue, Jan 30, 2018 at 4:48 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: > On Wed, Jan 31, 2018 at 6:01 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: >> On Tue, Jan 30, 2018 at 2:36 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >>> Duy Nguyen <pclouds@xxxxxxxxx> writes: >>> >>>> Is it simpler (though hacky) to just do >>>> >>>> #ifdef __cplusplus >>>> #define new not_new >>>> #define try really_try >>>> ... >>>> >>>> somewhere in git-compat-util.h? >>> >>> Very tempting, especially given that your approach automatically >>> would cover topics in flight without any merge conflict ;-) >>> >>> I agree that it is hacky and somewhat ugly, but the hackiness >>> somehow does not bother me too much in this case; perhaps because >>> attempting to use a C++ compiler may already be hacky in the first >>> place? >>> >>> It probably depends on the reason why we are doing this topic. If a >>> report about our source code coming from the C++ oriented tool cite >>> the symbol names seen by machines, then the "hacky" approach will >>> give us "not_new" where Brandon's patch may give us "new_oid", or >>> whatever symbol that is more appropriate for the context it appears >>> than such an automated cute name. > > Well. the world after post processing is always ugly. But we could try > "#define new new__" to get the not so ugly names. new_oid is > definitely better regardless of c/c++ though so I could see that as a > good cleanup. > >>>> Do we use any C features that are incompatible with C++? (or do we not >>>> need to care?) >>> >>> Good question. >> >> implicit casts from void? >> e.g. xmalloc returns a void pointer, not the type requested. >> https://embeddedartistry.com/blog/2017/2/28/c-casting-or-oh-no-we-broke-malloc > > That causes lots of warnings but not errors (I bit the bullet and > tried to compile git with g++). And for g++ there is a flag to disable this specific set of warnings. I think the value of using C++ analysis tools is in the LLVM/clang world, not GNU. > The next set of changes would be to > reorganize nested enum/struct declarations. Even if nested, C > considers these flat while C++ sees them in namespaces. There's some > warnings about confusion with the new cool feature string literals, > but that's easy to fix. > > There's also C99 designator in builtin/clean.c (I thought we avoided > C99, I can start using this specific feature more now :D) That was a test balloon? See 512f41cfac (clean.c: use designated initializer, 2017-07-14) One of the big advantages would be stricter type checking, such as signed/unsigned confusion, that we occasionally have. e.g. 61d36330b4 (prefer "!=" when checking read_in_full() result, 2017-09-27) or what is referenced from there 561598cfcf (read_pack_header: handle signed/unsigned comparison in read result, 2017-09-13). The bugs resulting in these patches could have been caught more easily with C++ checking IMHO. > I was stuck at the thread_local thing in index-pack.c and gave up. So > I don't know what else we would need to change. Thanks for experimenting! Stefan > -- > Duy
