From: Andi Kleen <andi@firstfloor.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Andi Kleen <andi@firstfloor.org>,
git@vger.kernel.org, Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH 1/2] Remove noreturn function pointers in usage.c
Date: Thu, 9 Jun 2011 06:59:15 +0200 [thread overview]
Message-ID: <20110609045915.GA15448@one.firstfloor.org> (raw)
In-Reply-To: <7v4o3z264s.fsf@alter.siamese.dyndns.org>
> - There are many more NORETURN and NORETURN_PTR in the code, and the
> proposed commit log message does not explain why these two are the only
> ones that are problematic and needs to be worked around. It does not
> guide other people who might want to add NORETURN/NORETURN_PTR when
> deciding if their change would break the "fix" this change brought in.
This was the only place where it crashed the compiler.
I don't have a good criterium to decide which cases do crash the compiler
or not except for trying it.
But I believe the crash is relatively unlikely (needs quite a lot of conditions
to line up), so it doesn't deserve extensive changes all over.
>
> - Potential impact to people who do not use Gcc 4.6 with profile feedback
> is not explained away well, except for "Doesn't seem to make any
> difference."
I merely went by "there are no new warnings" (I assume that's the main
motivation)
>
> - If other NORETURN/NORETURN_PTR could/should also go (I don't know due
> to the first bullet point above) when using the problematic compiler
> with the profile feedback feature, wouldn't it be a better workaround
> would be to introduce a Makefile variable to ask git-compat-util.h to
> make these two a no-op, perhaps?
I don't think we need to remove the others for now.
>
> A patch to do so may look like this.
>
> I did not like the triple negation "make NO_NORETURN=NoThanks" and wanted
> to name this AVOID_NORETURN instead, but decided to go with other existing
> Makefile variables.
Given the explanation above (I can update the description with that)
do you still want the complete disabling?
-Andi
next prev parent reply other threads:[~2011-06-09 4:59 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-08 21:43 [PATCH 1/2] Remove noreturn function pointers in usage.c Andi Kleen
2011-06-08 21:43 ` [PATCH 2/2] Add profile feedback build to git Andi Kleen
2011-06-09 0:36 ` [PATCH 1/2] Remove noreturn function pointers in usage.c Junio C Hamano
2011-06-09 4:59 ` Andi Kleen [this message]
2011-06-09 5:52 ` Jeff King
2011-06-09 6:31 ` Andi Kleen
2011-06-09 21:13 ` Erik Faye-Lund
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110609045915.GA15448@one.firstfloor.org \
--to=andi@firstfloor.org \
--cc=ak@linux.intel.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.