{"thread":{"id":"24617","subject":"[RFC/PATCH] git-compat-util.h: Don't define NORETURN under __clang__","startedAt":"2010-08-03T13:08:03Z","lastAt":"2010-08-03T21:10:34Z","messageCount":6,"participants":["Ævar Arnfjörð Bjarmason","Michael J Gruber","Benjamin Kramer","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"147044","messageId":"1280840883-24540-1-git-send-email-avarab@gmail.com","threadId":"24617","inReplyTo":null,"subject":"[RFC/PATCH] git-compat-util.h: Don't define NORETURN under __clang__","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-03T13:08:03Z","receivedAt":"2010-08-03T13:08:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"clang version 1.0 on Debian testing x86_64 defines __GNUC__, but barfs\non `void __attribute__((__noreturn__))'. E.g.:\n\n    usage.c:56:1: error: function declared 'noreturn' should not return [-Winvalid-noreturn]\n    }\n    ^\n    1 diagnostic generated.\n    make: *** [usage.o] Error 1\n\nThere it's dying on `void __attribute__((__noreturn__)) usagef(const\nchar *err, ...)' in usage.c, which doesn't return.\n\nChange the header to define NORETURN to nothing under clang. This was\nthe default behavior for non-GNU and non-MSC compilers already. Having\nNORETURN_PTR defined to the GNU C value has no effect on clang\nhowever.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nI have no experience with Clang so this may not be sane, but on my\nsystem clang compiles with it and passes all tests. It still spews a\nlot of warnings though:\n    \n    GITGUI_VERSION = 0.12.0.64.g89d61\n        * new build flags or prefix\n    config.c:297:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    connect.c:151:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    date.c:678:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    diff.c:429:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n\nThese are valid, as control may not return the advertised type if we\ncall die() inside them.\n\n    log-tree.c:297:21: warning: field width should have type 'int', but argument has type 'unsigned int' [-Wformat]\n                             \"Subject: [%s %0*d/%d] \",\n                                             ^\n    1 diagnostic generated.\n    notes.c:632:25: warning: field precision should have type 'int', but argument has type 'unsigned int' [-Wformat]\n            strbuf_addf(buf, \"%o %.*s%c\", mode, path_len, path, '\\0');\n                                   ^            ~~~~~~~~\n    1 diagnostic generated.\n\nShould these (and some below) just cast to (int) ?\n\n    object.c:44:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    parse-options.c:155:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    read-cache.c:1361:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    remote.c:658:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    revision.c:253:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    setup.c:79:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    sideband.c:97:25: warning: field precision should have type 'int', but argument has type 'unsigned int' [-Wformat]\n                                            fprintf(stderr, \"%.*s\", brk + sf, b);\n                                                               ^    ~~~~~~~~\n    1 diagnostic generated.\n    transport.c:1133:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    imap-send.c:548:27: warning: more data arguments than '%' conversions [-Wformat-extra-args]\n                               cmd->tag, cmd->cmd, cmd->cb.dlen);\n                                                   ^\n    1 diagnostic generated.\n    Writing perl.mak for Git\n    GIT_VERSION = 1.7.2.1.7.gb44c1\n    builtin/blame.c:1984:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    builtin/bundle.c:67:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    builtin/commit.c:832:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    builtin/fetch-pack.c:209:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    builtin/grep.c:704:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    builtin/help.c:58:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    builtin/pack-redundant.c:584:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    builtin/push.c:252:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    builtin/reflog.c:782:1: warning: control may reach end of non-void function [-Wreturn-type]\n    }\n    ^\n    1 diagnostic generated.\n    \n git-compat-util.h |    5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 02a73ee..c651cb7 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -183,7 +183,10 @@ extern char *gitbasename(char *);\n #define is_dir_sep(c) ((c) == '/')\n #endif\n \n-#ifdef __GNUC__\n+#ifdef __clang__\n+#define NORETURN\n+#define NORETURN_PTR __attribute__((__noreturn__))\n+#elif __GNUC__\n #define NORETURN __attribute__((__noreturn__))\n #define NORETURN_PTR __attribute__((__noreturn__))\n #elif defined(_MSC_VER)\n-- \n1.7.1\n"},{"id":"147047","messageId":"4C581731.7060808@drmicha.warpmail.net","threadId":"24617","inReplyTo":"1280840883-24540-1-git-send-email-avarab@gmail.com","subject":"Re: [RFC/PATCH] git-compat-util.h: Don't define NORETURN under __clang__","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-08-03T13:18:41Z","receivedAt":"2010-08-03T13:18:41Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Ævar Arnfjörð Bjarmason venit, vidit, dixit 03.08.2010 15:08:\n> clang version 1.0 on Debian testing x86_64 defines __GNUC__, but barfs\n> on `void __attribute__((__noreturn__))'. E.g.:\n> \n>     usage.c:56:1: error: function declared 'noreturn' should not return [-Winvalid-noreturn]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     make: *** [usage.o] Error 1\n> \n> There it's dying on `void __attribute__((__noreturn__)) usagef(const\n> char *err, ...)' in usage.c, which doesn't return.\n> \n> Change the header to define NORETURN to nothing under clang. This was\n> the default behavior for non-GNU and non-MSC compilers already. Having\n> NORETURN_PTR defined to the GNU C value has no effect on clang\n> however.\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n> \n> I have no experience with Clang so this may not be sane, but on my\n> system clang compiles with it and passes all tests. It still spews a\n> lot of warnings though:\n>     \n>     GITGUI_VERSION = 0.12.0.64.g89d61\n>         * new build flags or prefix\n>     config.c:297:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     connect.c:151:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     date.c:678:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     diff.c:429:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n> \n> These are valid, as control may not return the advertised type if we\n> call die() inside them.\n> \n>     log-tree.c:297:21: warning: field width should have type 'int', but argument has type 'unsigned int' [-Wformat]\n>                              \"Subject: [%s %0*d/%d] \",\n>                                              ^\n>     1 diagnostic generated.\n>     notes.c:632:25: warning: field precision should have type 'int', but argument has type 'unsigned int' [-Wformat]\n>             strbuf_addf(buf, \"%o %.*s%c\", mode, path_len, path, '\\0');\n>                                    ^            ~~~~~~~~\n>     1 diagnostic generated.\n> \n> Should these (and some below) just cast to (int) ?\n> \n>     object.c:44:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     parse-options.c:155:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     read-cache.c:1361:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     remote.c:658:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     revision.c:253:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     setup.c:79:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     sideband.c:97:25: warning: field precision should have type 'int', but argument has type 'unsigned int' [-Wformat]\n>                                             fprintf(stderr, \"%.*s\", brk + sf, b);\n>                                                                ^    ~~~~~~~~\n>     1 diagnostic generated.\n>     transport.c:1133:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     imap-send.c:548:27: warning: more data arguments than '%' conversions [-Wformat-extra-args]\n>                                cmd->tag, cmd->cmd, cmd->cb.dlen);\n>                                                    ^\n>     1 diagnostic generated.\n>     Writing perl.mak for Git\n>     GIT_VERSION = 1.7.2.1.7.gb44c1\n>     builtin/blame.c:1984:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     builtin/bundle.c:67:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     builtin/commit.c:832:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     builtin/fetch-pack.c:209:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     builtin/grep.c:704:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     builtin/help.c:58:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     builtin/pack-redundant.c:584:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     builtin/push.c:252:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     builtin/reflog.c:782:1: warning: control may reach end of non-void function [-Wreturn-type]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     \n>  git-compat-util.h |    5 ++++-\n>  1 files changed, 4 insertions(+), 1 deletions(-)\n> \n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 02a73ee..c651cb7 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -183,7 +183,10 @@ extern char *gitbasename(char *);\n>  #define is_dir_sep(c) ((c) == '/')\n>  #endif\n>  \n> -#ifdef __GNUC__\n> +#ifdef __clang__\n> +#define NORETURN\n> +#define NORETURN_PTR __attribute__((__noreturn__))\n> +#elif __GNUC__\n\n__GNUC__ should be true if defined, but maybe you still want\n\n+#elif defined(__GNUC__)\n\ninstead to make this really equivalent in the GNUC case.\n\n>  #define NORETURN __attribute__((__noreturn__))\n>  #define NORETURN_PTR __attribute__((__noreturn__))\n>  #elif defined(_MSC_VER)\n\nMichael\n"},{"id":"147053","messageId":"vpq62zr24zw.fsf@bauges.imag.fr","threadId":"24617","inReplyTo":"1280840883-24540-1-git-send-email-avarab@gmail.com","subject":"Re: [RFC/PATCH] git-compat-util.h: Don't define NORETURN under __clang__","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-03T13:35:47Z","receivedAt":"2010-08-03T13:35:47Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ævar Arnfjörð Bjarmason venit, vidit, dixit 03.08.2010 15:08:\n\n> clang version 1.0 on Debian testing x86_64 defines __GNUC__, but barfs\n> on `void __attribute__((__noreturn__))'. E.g.:\n> \n>     usage.c:56:1: error: function declared 'noreturn' should not return [-Winvalid-noreturn]\n>     }\n>     ^\n>     1 diagnostic generated.\n>     make: *** [usage.o] Error 1\n\nIt doesn't mean that it's not accepting __noreturn__, it means it was\nnot smart enough to check that the function do not return.\n\nIn my git, usage.c:56: leads me to this function:\n\nvoid usagef(const char *err, ...)\n{\n\tva_list params;\n\n\tva_start(params, err);\n\tusage_routine(err, params);\n\tva_end(params);\n}\n\nThe absence of return comes from usage_routine, which is a pointer to\nfunction, and it seems your version of clang doesn't handle\n__noreturn__ pointers to functions properly.\n\nOn my box:\n\ngit$ make -B CC=clang V=yes usage.o                                                                                                        \nclang -o usage.o -c   -g -Wall -Werror -I.  -DHAVE_PATHS_H -DSHA1_HEADER='<openssl/sha.h>'  -DNO_STRLCPY -DNO_MKSTEMPS  usage.c\ngit$ clang --version\nclang version 1.1 (branches/release_27)\nTarget: i386-pc-linux-gnu\nThread model: posix\n\nso, more recent clang do not seem to have this issue.\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 02a73ee..c651cb7 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -183,7 +183,10 @@ extern char *gitbasename(char *);\n>  #define is_dir_sep(c) ((c) == '/')\n>  #endif\n>  \n> -#ifdef __GNUC__\n> +#ifdef __clang__\n> +#define NORETURN\n> +#define NORETURN_PTR __attribute__((__noreturn__))\n> +#elif __GNUC__\n\nIf you go for something like this, you should check the version of\nclang, and special-case only version < 1.1.\n\nBut I'm not sure special-casing old version of a young compiler really\nmakes sense. We're only talking about warnings here, so I'd say you\nshould either upgrade clang or remove -Werror from your CFLAGS.\n\n(other than that, it's cool to see someone testing another\ncompiler ;-) )\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"147052","messageId":"C210797E-AE95-4074-AADE-CD2B518D8202@gmail.com","threadId":"24617","inReplyTo":"1280840883-24540-1-git-send-email-avarab@gmail.com","subject":"Re: [RFC/PATCH] git-compat-util.h: Don't define NORETURN under __clang__","fromName":"Benjamin Kramer","fromEmail":"benny.kra@googlemail.com","sentAt":"2010-08-03T13:35:51Z","receivedAt":"2010-08-03T13:35:51Z","isPatch":true,"sender":{"key":"benny.kra@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/16542?v=4"},"body":"\nOn 03.08.2010, at 15:08, Ævar Arnfjörð Bjarmason wrote:\n\n> clang version 1.0 on Debian testing x86_64 defines __GNUC__, but barfs\n> on `void __attribute__((__noreturn__))'. E.g.:\n\nThis is a bug in clang 1.0 that was fixed in the llvm 2.7/clang 1.5 timeframe, it didn't\nrecognize attributes on function pointers properly. clang tries to be as compatible with GCC as\npossible so it obviously has to define __GNUC__ ;)\n\n> \n>    usage.c:56:1: error: function declared 'noreturn' should not return [-Winvalid-noreturn]\n>    }\n>    ^\n>    1 diagnostic generated.\n>    make: *** [usage.o] Error 1\n\nIt's a \"warning which defaults to an error\" you can pacify it by passing -Winvalid-noreturn or\n\u0010-Wno-invalid-noreturn.\n\nI oppose adding workarounds for obsolete versions of clang, Debian should upgrade their packages to\na more recent release. clang 1.0 had all kinds of weird bugs and shouldn't be used anymore."},{"id":"147055","messageId":"vpqvd7rzsfa.fsf@bauges.imag.fr","threadId":"24617","inReplyTo":"vpq62zr24zw.fsf@bauges.imag.fr","subject":"Re: [RFC/PATCH] git-compat-util.h: Don't define NORETURN under __clang__","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-03T14:23:21Z","receivedAt":"2010-08-03T14:23:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> (other than that, it's cool to see someone testing another\n> compiler ;-) )\n\nBTW, the only warnings remaining with -Wall with my clang are:\n\nimap-send.c:548:27: warning: data argument not used by format string [-Wformat-extra-args]\n                           cmd->tag, cmd->cmd, cmd->cb.dlen);\n                                               ^\nimap-send.c:1089:41: warning: conversion specifies type 'unsigned short' but the argument has type 'int' [-Wformat]\n                snprintf(portstr, sizeof(portstr), \"%hu\", srvc->port);\n                                                    ~~^   ~~~~~~~~~~\n2 diagnostics generated.\n\nand the tests pass.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"147072","messageId":"AANLkTinNkLNGjFrfmo5za_D10AkcMEMzA8yppA+H+YMe@mail.gmail.com","threadId":"24617","inReplyTo":"vpqvd7rzsfa.fsf@bauges.imag.fr","subject":"Re: [RFC/PATCH] git-compat-util.h: Don't define NORETURN under __clang__","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-03T21:10:34Z","receivedAt":"2010-08-03T21:10:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Thanks everyone. I should have tested a later version of clang before\nI sent the patch. It might still be worthwhile to munge the flags for\nold clangs so that git doesn't error out on it, but if 1.0 doesn't\nmake it into some major OS release and gcc remains the default it's\nnot much of an issue.\n\nOn Tue, Aug 3, 2010 at 14:23, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> (other than that, it's cool to see someone testing another\n>> compiler ;-) )\n>\n> BTW, the only warnings remaining with -Wall with my clang are:\n>\n> imap-send.c:548:27: warning: data argument not used by format string [-Wformat-extra-args]\n>                           cmd->tag, cmd->cmd, cmd->cb.dlen);\n>                                               ^\n\nI didn't look into that one. It'd be usefu to check out if the format\nstring is really incomplete there, or if clang is just failing it its\nanalysis.\n\nThat's some hairy code, in any case.\n\n> imap-send.c:1089:41: warning: conversion specifies type 'unsigned short' but the argument has type 'int' [-Wformat]\n>                snprintf(portstr, sizeof(portstr), \"%hu\", srvc->port);\n>                                                    ~~^   ~~~~~~~~~~\n> 2 diagnostics generated.\n\nLooks like that needs a cast.\n"}]}