Daniel Shahaf wrote on Thu, Aug 01, 2013 at 05:47:58 +0300:
> Stefan Fuhrmann wrote on Wed, Jul 31, 2013 at 23:21:45 +0200:
> > * revert svn_hash_gets to simply use APR_HASH_KEY_STRING
>
> ...
>
> > * where it is being used, make svn_private_config.h the first #include
>
> Why would the order of includes matter? Do you plan to still use
> SVN_HAS_DUNDER_BUILTINS in svn_hash_sets()?
Personally I'd simply revert back to the code right now on the 1.8
branch:
#define svn_hash_gets(ht, key) \
apr_hash_get(ht, key, APR_HASH_KEY_STRING)
#define svn_hash_sets(ht, key, val) \
apr_hash_set(ht, key, APR_HASH_KEY_STRING, val)
and strip all the dunder-builtins stuff.
[[[
Index: subversion/include/svn_hash.h
===================================================================
--- subversion/include/svn_hash.h (revision 1509078)
+++ subversion/include/svn_hash.h (working copy)
@@ -239,52 +239,19 @@ svn_hash_from_cstring_keys(apr_hash_t **hash,
const apr_array_header_t *keys,
apr_pool_t *pool);
-/* Used by the svn_hash_gets macro.
- * Make sure you either defined it before including this header or not
- * at all.
- */
-#if !defined(SVN_HAS_DUNDER_BUILTINS)
-/* If you define it elsewhere to generate better code, the definition
- * below will usually generate a compiler warning if your definition is
- * being encountered _after_ #including this header.
- */
-#define SVN_HAS_DUNDER_BUILTINS 0
-#endif
-
/** Shortcut for apr_hash_get() with a const char * key.
*
* @since New in 1.8.
*/
-#if SVN_HAS_DUNDER_BUILTINS
-/* We have three use-cases:
- 1. (common) KEY is a string literal.
- 2. (less common) KEY is a const char*.
- 3. (rare) KEY is the result of an expensive function call.
- For the first, we want to evaluate the string length at compile time.
- (We use strlen(), which gets optimized to a constant.) For the other
- two, however, we want to avoid having the macro multiply-evaluate KEY.
- So, if our compiler is smart enough (that includes at least gcc and
- clang), we use __builtin_constant_p() to have our cake and eat it too: */
-# define svn_hash_gets(ht, key) \
- apr_hash_get(ht, key, \
- __builtin_constant_p(strlen(key)) \
- ? strlen(key) : APR_HASH_KEY_STRING)
-#else
-/* ... and when our compiler is anything else, we take the hit and compute the
- length of string literals at run-time. */
-# define svn_hash_gets(ht, key) \
+#define svn_hash_gets(ht, key) \
apr_hash_get(ht, key, APR_HASH_KEY_STRING)
-#endif
/** Shortcut for apr_hash_set() with a const char * key.
*
* @since New in 1.8.
*/
-#define svn_hash_sets(ht, key, val) \
- do { \
- const void *svn_key__temp = (key); \
- apr_hash_set(ht, svn_key__temp, strlen(svn_key__temp), val); \
- } while (0)
+#define svn_hash_sets(ht, key, val) \
+ apr_hash_set(ht, key, APR_HASH_KEY_STRING, val)
/** @} */
]]]
[[[
Index: build/ac-macros/svn-macros.m4
===================================================================
--- build/ac-macros/svn-macros.m4 (revision 1509078)
+++ build/ac-macros/svn-macros.m4 (working copy)
@@ -163,25 +163,3 @@ AC_DEFUN([SVN_CHECK_FOR_ATOMIC_BUILTINS],
return 0;
}], [svn_cv_atomic_builtins=yes], [svn_cv_atomic_builtins=no],
[svn_cv_atomic_builtins=no])])
])
-
-AC_DEFUN([SVN_CHECK_FOR_DUNDER_BUILTINS],
-[
- AC_CACHE_CHECK([whether the compiler provides dunder builtins],
[svn_cv_dunder_builtins],
- [
- AC_RUN_IFELSE([AC_LANG_SOURCE([[
- int main(int argc)
- {
- return (!__builtin_constant_p(argc) && __builtin_constant_p("foobar"))
- ? 0 /* EXIT_SUCCESS */ : 1 /* EXIT_FAILURE */;
- }]])],
- svn_cv_dunder_builtins="yes",
- svn_cv_dunder_builtins="no",
- [AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
- int main(void){
- return __builtin_constant_p("foobar") ? 0 : 1;
- }]])],
- svn_cv_dunder_builtins="yes",
- svn_cv_dunder_builtins="no")
- ])
- ])
-])
Index: configure.ac
===================================================================
--- configure.ac (revision 1509078)
+++ configure.ac (working copy)
@@ -171,11 +171,6 @@ if test "$svn_cv_atomic_builtins" = "yes"; then
AC_DEFINE(SVN_HAS_ATOMIC_BUILTINS, 1, [Define if compiler provides atomic
builtins])
fi
-SVN_CHECK_FOR_DUNDER_BUILTINS
-if test "$svn_cv_dunder_builtins" = "yes"; then
- AC_DEFINE(SVN_HAS_DUNDER_BUILTINS, 1, [Define if compiler provides
__builtin_constant_p])
-fi
-
dnl Set up a number of directories ---------------------
dnl Create SVN_BINDIR for proper substitution
Index: subversion/svn_private_config.hw
===================================================================
--- subversion/svn_private_config.hw (revision 1509078)
+++ subversion/svn_private_config.hw (working copy)
@@ -48,9 +48,6 @@
#define SVN_FS_WANT_DB_MINOR 0
#define SVN_FS_WANT_DB_PATCH 14
-/* No support for __builtin_constant_p */
-#define SVN_HAS_DUNDER_BUILTINS 0
-
/* Path separator for local filesystem */
#define SVN_PATH_LOCAL_SEPARATOR '\\'
]]]
