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; commits_at_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 <danielsh_at_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 <danielsh_at_elego.de>
> > wrote:
> > >
> > > > danielsh_at_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.
Received on 2013-07-30 18:04:52 CEST