build/ac-macros/svn-macros.m4 configure.ac subversion/include/svn_hash.h MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable
If this really makes a measurable difference for the average user (which I doubt it even is for our use) this belongs in Apr. Until then I think we are wasting a lot of development time in optimizing a non measurable piece of our code, with unconfirmed 'improvements' that time after time cause new problems. Bert From: Daniel Shahaf Sent: =E2=80=8E30/=E2=80=8E07/=E2=80=8E2013 16:47 To: Stefan Fuhrmann Cc: Subversion Development; comm...@subversion.apache.org Subject: Re: svn commit: r1508170 - in /subversion/trunk: build/ac-macros/svn-macros.m4 configure.ac subversion/include/svn_hash.h Stefan Fuhrmann wrote on Tue, Jul 30, 2013 at 16:30:00 +0200: > On Tue, Jul 30, 2013 at 3:57 PM, Daniel Shahaf <danie...@elego.de> wrote: >=20 > > Stefan Fuhrmann wrote on Tue, Jul 30, 2013 at 01:04:43 +0200: > > > On Mon, Jul 29, 2013 at 9:06 PM, Daniel Shahaf <danie...@elego.de> > > wrote: > > > > > > > danie...@apache.org wrote on Mon, Jul 29, 2013 at 18:35:26 -0000: > > > > > Author: danielsh > > > > > Date: Mon Jul 29 18:35:25 2013 > > > > > New Revision: 1508170 > > > > > > > > > > URL: http://svn.apache.org/r1508170 > > > > > Log: > > > > > Follow-up to r1507366: svn_hash_gets: compute the length of > > > > > string literal keys (common case) at compile-time, without > > > > > multiply-evaluating dynamically-computed keys. > > > > > > > > > > Review by: philip > > > > > breser > > > > > > > > Oddly enough, the optimization doesn't seem to kick in for me ... > > > > > > > > > > A two things have been broken here: > > > > > > > Thanks for fixing this. Collective review of your commits: > > > > r1508222: the #ifndef seems to be the wrong solution to whatever proble= m > > it is trying to fix. If it's trying to generate a compiler warning, it > > should use the #warning preprocessor directive; if it's trying to > > silence one about use of an undefined macro, how about doing > > #ifndef SVN_HAS_DUNDER_BUILTINS > > #error "Build broken (do you need to include svn_private_config.h ear= lier?)" > > #endif > > >=20 > No go. This is a public API header. People will not define > SVN_HAS_DUNDER_BUILTINS in their applications. >=20 It would be nice if we can propagate the fix to C API consumers too. How can we do that? Do we just document the macro (say, in an #ifdef DOXYGEN block) along with an example of how to detect the setting for it in various build systems? Or do try to sniff it at compile time? > > r1508225: changing the include order //only in places that currently > > include svn_hash.h// seems like a bad idea; we should change it > > _everywhere_ (since other places may grow svn_hash.h includes in the > > future). > > >=20 > Well, I got kind of fed up after manually patching > ~180 files (about the same number of files left to > check). Even the commit took longer than 4 minutes. >=20 > So, feel free to do some search/replace magic to > handle the remaining files. What makes you think I'm not as lazy as you are? :-) Index: build/ac-macros/compiler.m4 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- build/ac-macros/compiler.m4=09(revision 1508471) +++ build/ac-macros/compiler.m4=09(working copy) @@ -74,6 +74,7 @@ AC_DEFUN([SVN_CC_MODE_SETUP], =20 CNOWARNFLAGS=3D"$CFLAGS" CFLAGS=3D"$CFLAGS_KEEP" + CFLAGS=3D"-DSVN__CORE $CFLAGS" =20 AC_SUBST(CMODEFLAGS) AC_SUBST(CNOWARNFLAGS) Index: build/generator/gen_win.py =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- build/generator/gen_win.py=09(revision 1508471) +++ build/generator/gen_win.py=09(working copy) @@ -714,7 +714,8 @@ class WinGeneratorBase(gen_win_dependencies.GenDep fakedefines =3D ["WIN32","_WINDOWS","alloca=3D_alloca", "_CRT_SECURE_NO_DEPRECATE=3D", "_CRT_NONSTDC_NO_DEPRECATE=3D", - "_CRT_SECURE_NO_WARNINGS=3D"] + "_CRT_SECURE_NO_WARNINGS=3D", + "SVN__CORE"] =20 if cfg =3D=3D 'Debug': fakedefines.extend(["_DEBUG","SVN_DEBUG"]) Index: subversion/include/svn_hash.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- subversion/include/svn_hash.h=09(revision 1508471) +++ subversion/include/svn_hash.h=09(working copy) @@ -248,7 +248,11 @@ svn_hash_from_cstring_keys(apr_hash_t **hash, * below will usually generate a compiler warning if your definition is * being encountered _after_ #including this header.=20 */ -#define SVN_HAS_DUNDER_BUILTINS 0 +# ifdef SVN__CORE +# error "Build broken (should you include svn_private_config.h earlier?)" +# else +# define SVN_HAS_DUNDER_BUILTINS 0 +# endif #endif =20 /** Shortcut for apr_hash_get() with a const char * key. Untested, but I'll add it to my queue for next week. > > Also, let's take this opportunity to document our preferred order of > > #include's in HACKING (The relative order of: stdlib, apr, mod_dav, bdb= , > > serf, sqlite, svn_*.h, svn_*_private.h, ./*.h, ../libsvn_{fs|ra}/*.h, > > subversion_config_private.h, and APR_WANT_FOO) --- I always end > > up copying the order from some existing file.
