Re: [PATCH] Add the GIT_SENTINEL macro
Junio C Hamano wrote: Ramsay Jones ram...@ramsay1.demon.co.uk writes: The sentinel function attribute is not understood by versions of the gcc compiler prior to v4.0. At present, for earlier versions of gcc, the build issues 108 warnings related to the unknown attribute. In order to suppress the warnings, we conditionally define the GIT_SENTINEL macro to provide the sentinel attribute for gcc v4.0 and newer. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- This was built on the next branch It seems to apply well on the tip of jk/gcc-function-attributes. - This macro is not about git at all, so I'll edit the patch to call it GCC_ATTR_SENTINEL before applying. - Also I'll drop the (0) at the end. Thanks. Yes, GCC_ATTR_SENTINEL is a better name, but I like LAST_ARG_MUST_BE_NULL even more! The final commit 9fe3edc4 (Add the LAST_ARG_MUST_BE_NULL macro, 18-07-2013) is much better than my patch and works beautifully. Thanks Junio, Jeff and Jonathan! ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add the GIT_SENTINEL macro
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: It seems to apply well on the tip of jk/gcc-function-attributes. - This macro is not about git at all, so I'll edit the patch to call it GCC_ATTR_SENTINEL before applying. Would naming it something like LAST_ARG_MUST_BE_NULL instead make sense? That way, if some other compiler gains a different syntax for the same annotation, it would be possible to do #if defined(__GNUC__) __GNUC__ = 4 # define LAST_ARG_MUST_BE_NULL __attribute__((sentinel)) #elif defined(_MSC_VER) _MSC_VER 27 # define LAST_ARG_MUST_BE_NULL __declspec(lastargnull) #else # define LAST_ARG_MUST_BE_NULL #endif I do like last-arg-must-be-null name for its descriptiveness; with the example of NORETURN vs NORETURN_PTR, however, I am not quite convinced that it would help reusing the same macro to different compilers. I would say it is already sheer luck that GCC's __attribute__(()) and MSC's __declspec() both come at the same place in the function declaration syntax, i.e. before the usual declaration. Your next compiler may want the magic immediately before the closing semicolon of the declaration, for example. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add the GIT_SENTINEL macro
On Thu, Jul 18, 2013 at 09:02:12PM +0100, Ramsay Jones wrote: The sentinel function attribute is not understood by versions of the gcc compiler prior to v4.0. At present, for earlier versions of gcc, the build issues 108 warnings related to the unknown attribute. In order to suppress the warnings, we conditionally define the GIT_SENTINEL macro to provide the sentinel attribute for gcc v4.0 and newer. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- Acked-by: Jeff King p...@peff.net -__attribute__((sentinel)) +GIT_SENTINEL(0) void argv_array_pushl(struct argv_array *, ...); We could also add GIT_SENTINEL to handle the default-zero case, but I do not think it is worth the trouble. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add the GIT_SENTINEL macro
Ramsay Jones wrote: This was built on the next branch All the uses of the sentinel attribute seem to come from eccb6149 (use sentinel function attribute for variadic lists, 2013-07-09), so this should be okay to go on top of the jk/gcc-function-attributes branch. --- a/git-compat-util.h +++ b/git-compat-util.h @@ -303,6 +303,13 @@ extern char *gitbasename(char *); #endif #endif +/* The sentinel attribute is valid from gcc version 4.0 */ +#if defined(__GNUC__) (__GNUC__ = 4) +#define GIT_SENTINEL(n) __attribute__((sentinel(n))) +#else +#define GIT_SENTINEL(n) +#endif I'd mildly prefer #if ... #define GIT_SENTINEL __attribute__((sentinel)) #else ... (without the numeric parameter). I don't know any function in git (or any other project for that matter) that takes extra parameters after the NULL sentinel. But I don't care much, so Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks for a pleasant patch. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add the GIT_SENTINEL macro
Ramsay Jones ram...@ramsay1.demon.co.uk writes: The sentinel function attribute is not understood by versions of the gcc compiler prior to v4.0. At present, for earlier versions of gcc, the build issues 108 warnings related to the unknown attribute. In order to suppress the warnings, we conditionally define the GIT_SENTINEL macro to provide the sentinel attribute for gcc v4.0 and newer. Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk --- This was built on the next branch It seems to apply well on the tip of jk/gcc-function-attributes. - This macro is not about git at all, so I'll edit the patch to call it GCC_ATTR_SENTINEL before applying. - Also I'll drop the (0) at the end. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add the GIT_SENTINEL macro
Junio C Hamano wrote: It seems to apply well on the tip of jk/gcc-function-attributes. - This macro is not about git at all, so I'll edit the patch to call it GCC_ATTR_SENTINEL before applying. Would naming it something like LAST_ARG_MUST_BE_NULL instead make sense? That way, if some other compiler gains a different syntax for the same annotation, it would be possible to do #if defined(__GNUC__) __GNUC__ = 4 # define LAST_ARG_MUST_BE_NULL __attribute__((sentinel)) #elif defined(_MSC_VER) _MSC_VER 27 # define LAST_ARG_MUST_BE_NULL __declspec(lastargnull) #else # define LAST_ARG_MUST_BE_NULL #endif -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
