Re: Git ChangeLog policy for GCC Testsuite inquiry
On 25/01/2020 16:11, Jeff Law wrote: On Sat, 2020-01-25 at 10:50 -0500, Nathan Sidwell wrote: On 1/24/20 4:36 PM, Jeff Law wrote: On Fri, 2020-01-24 at 20:32 +0100, Eric Botcazou wrote: I strongly prefer to move towards relying on the git log. In my experience the output of git log is a total mess so cannot replace ChangeLogs. But we can well decide to drop ChangeLog for the testsuite. Well, glibc has moved to extracting them from git, building policies and scripts around that. I'm pretty sure other significant projecs are also extracting their ChangeLogs from git. We could do the same, selecting some magic date as the cutover point after which future ChangeLogs are extracted from GIT. In fact, that's precisely what I'd like to see us do. The GCC10 release date would seem a good point to do this. That gives us around 3 months to figure the details (and get stakeholder buy-in) Yup. That would be what I'd recommend we shoot for. As you say it gives time to work on the details and for folks to start changing their habits. jeff What about the maintained branches? Do they change as well? or would they keep the existing ChangeLog files? If the latter, then backports will need more care, but if the former, then the cut-over date needs careful thought as they follow a different cadence. R.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On 22/01/2020 17:45, Richard Earnshaw (lists) wrote: [updated based on v2 discussions] This patch proposes some new (additional) rules for email subject lines when contributing to GCC. The goal is to make sure that, as far as possible, the subject for a patch will form a good summary when the message is committed to the repository if applied with 'git am'. Where possible, I've tried to align these rules with those already in use for glibc, so that the differences are minimal and only where necessary. Some things that differ from existing practice (at least by some people) are: - Use ':' rather than '[]' - This is more git friendly and works with 'git am'. - Put bug numbers at the end of the line rather than the beginning. - The bug number is useful, but not as useful as the brief summary. Also, use the shortened form, as the topic part is more usefully conveyed in the proper topic field (see above). I've not seen any follow-up to this version. Should we go ahead and adopt this? R.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > I've not seen any follow-up to this version. Should we go ahead and adopt > this? Can we please go with 'committed' (lowercase) rather than all-caps COMMITTED? Spelling this with all-caps seems like a recent thing on gcc-patches, before everyone used the lowercase version, which makes more sense (no need to shout about the thing that didn't need any discussion before applying the patch). Also, while tools like 'git format-patch' will automatically put [PATCH] in the subject, for '[COMMITTED]' it will be the human typing that out, and it makes little sense to require people to meticulously type that out in caps. Especially when the previous practice was opposite. Thanks. Alexander
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On 03/02/2020 11:54, Alexander Monakov wrote: On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: I've not seen any follow-up to this version. Should we go ahead and adopt this? Can we please go with 'committed' (lowercase) rather than all-caps COMMITTED? Spelling this with all-caps seems like a recent thing on gcc-patches, before everyone used the lowercase version, which makes more sense (no need to shout about the thing that didn't need any discussion before applying the patch). Also, while tools like 'git format-patch' will automatically put [PATCH] in the subject, for '[COMMITTED]' it will be the human typing that out, and it makes little sense to require people to meticulously type that out in caps. Especially when the previous practice was opposite. Thanks. Alexander Upper case is what glibc has, though it appears that it's a rule that is not strictly followed. If we change it, then it becomes another friction point between developer groups. Personally, I'd leave it as is, then turn a blind eye to such minor non-conformance. Quite frankly, then bit that matters most is what follows that, since that is what gets written into the git commit message. R.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > Upper case is what glibc has, though it appears that it's a rule that is not > strictly followed. If we change it, then it becomes another friction point > between developer groups. Personally, I'd leave it as is, then turn a blind > eye to such minor non-conformance. In that case can we simply say that both 'committed' and 'COMMITTED' are okay, if we know glibc doesn't follow that rule and anticipate we will not follow it either? Thanks. Alexander
Re: Git ChangeLog policy for GCC Testsuite inquiry
On 2/3/20 5:15 AM, Richard Earnshaw (lists) wrote: On 25/01/2020 16:11, Jeff Law wrote: On Sat, 2020-01-25 at 10:50 -0500, Nathan Sidwell wrote: On 1/24/20 4:36 PM, Jeff Law wrote: On Fri, 2020-01-24 at 20:32 +0100, Eric Botcazou wrote: I strongly prefer to move towards relying on the git log. In my experience the output of git log is a total mess so cannot replace ChangeLogs. But we can well decide to drop ChangeLog for the testsuite. Well, glibc has moved to extracting them from git, building policies and scripts around that. I'm pretty sure other significant projecs are also extracting their ChangeLogs from git. We could do the same, selecting some magic date as the cutover point after which future ChangeLogs are extracted from GIT. In fact, that's precisely what I'd like to see us do. The GCC10 release date would seem a good point to do this. That gives us around 3 months to figure the details (and get stakeholder buy-in) Yup. That would be what I'd recommend we shoot for. As you say it gives time to work on the details and for folks to start changing their habits. jeff What about the maintained branches? Do they change as well? or would they keep the existing ChangeLog files? If the latter, then backports will need more care, but if the former, then the cut-over date needs careful thought as they follow a different cadence. I have a mild preference for release branches to follow trunk's behaviour -- that's easier to remember. But, I don't look after a release branch. I'm not sure why you think the release cadence affects that? Just add a top entry to all testsuite changelogs indicating it's no longer used. Perhaps I'm missing something. Other branches at the branch owner's discretion. nathan -- Nathan Sidwell
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, Feb 03, 2020 at 02:54:05PM +0300, Alexander Monakov wrote: > On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > > > I've not seen any follow-up to this version. Should we go ahead and adopt > > this? > > Can we please go with 'committed' (lowercase) rather than all-caps COMMITTED? > Spelling this with all-caps seems like a recent thing on gcc-patches, before > everyone used the lowercase version, which makes more sense (no need to shout > about the thing that didn't need any discussion before applying the patch). Lower case certainly makes more sense. None of this are *rules*. We should not pretend they are. An email subject should be useful to what the receivers of that email use it for: see if it very interesting to them, see if it probably not interesting to them: that's the "smth: " at the start, and the PR number at the end, and of course the actual subject itself, so we should not put in too much fluff in the subject, there needs to be room left (in the less than fifty chars total) for an actual subject :-) (The example in the patch does not capitalise the subject line, btw. It should.) Segher
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On 03/02/2020 13:54, Segher Boessenkool wrote: On Mon, Feb 03, 2020 at 02:54:05PM +0300, Alexander Monakov wrote: On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: I've not seen any follow-up to this version. Should we go ahead and adopt this? Can we please go with 'committed' (lowercase) rather than all-caps COMMITTED? Spelling this with all-caps seems like a recent thing on gcc-patches, before everyone used the lowercase version, which makes more sense (no need to shout about the thing that didn't need any discussion before applying the patch). Lower case certainly makes more sense. None of this are *rules*. We should not pretend they are. An email subject should be useful to what the receivers of that email use it for: see if it very interesting to them, see if it probably not interesting to them: that's the "smth: " at the start, and the PR number at the end, and of course the actual subject itself, so we should not put in too much fluff in the subject, there needs to be room left (in the less than fifty chars total) for an actual subject :-) (The example in the patch does not capitalise the subject line, btw. It should.) Segher Where does your '50 chars' limit come from? It's not in the glibc text, and it's not in the linux kernel text either. AFAICT this is your invention and you seem to be the only person proposing it. I think the linux rule (the whole line, not including the parts that are removed on commit, should not exceed 75 characters) is far more sensible - which is why this draft states this. R.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, Feb 3, 2020 at 7:57 AM Alexander Monakov wrote: > On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > > > Upper case is what glibc has, though it appears that it's a rule that is > not > > strictly followed. If we change it, then it becomes another friction > point > > between developer groups. Personally, I'd leave it as is, then turn a > blind > > eye to such minor non-conformance. > > In that case can we simply say that both 'committed' and 'COMMITTED' are > okay, > if we know glibc doesn't follow that rule and anticipate we will not follow > it either? > And perhaps something shorter? "committed" is a long word. [PUSHED]? Jason
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, 3 Feb 2020 at 14:00, Richard Earnshaw (lists) wrote: > Where does your '50 chars' limit come from? It's not in the glibc text, > and it's not in the linux kernel text either. AFAICT this is your > invention and you seem to be the only person proposing it. It's a fairly well established convention, e.g. https://chris.beams.io/posts/git-commit/ and it's what Github suggests (and whines if you go past it). > I think the linux rule (the whole line, not including the parts that are > removed on commit, should not exceed 75 characters) is far more sensible > - which is why this draft states this. I'm OK with that.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On 03/02/2020 14:13, Jonathan Wakely wrote: On Mon, 3 Feb 2020 at 14:00, Richard Earnshaw (lists) wrote: Where does your '50 chars' limit come from? It's not in the glibc text, and it's not in the linux kernel text either. AFAICT this is your invention and you seem to be the only person proposing it. It's a fairly well established convention, e.g. https://chris.beams.io/posts/git-commit/ and it's what Github suggests (and whines if you go past it). That suggest it as a limit for everything. If you have a tag and a bug number then then that would leave very little for the real part of the summary and would likely lead to something meaningless or incomprehensible in the remaining characters. That might be OK for small projects, but for something the size of gcc, I think keeping the extra flexibility is useful. I think the linux rule (the whole line, not including the parts that are removed on commit, should not exceed 75 characters) is far more sensible - which is why this draft states this. I'm OK with that. OK, thanks. R.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On 03/02/2020 14:10, Jason Merrill wrote: On Mon, Feb 3, 2020 at 7:57 AM Alexander Monakov wrote: On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: Upper case is what glibc has, though it appears that it's a rule that is not strictly followed. If we change it, then it becomes another friction point between developer groups. Personally, I'd leave it as is, then turn a blind eye to such minor non-conformance. In that case can we simply say that both 'committed' and 'COMMITTED' are okay, if we know glibc doesn't follow that rule and anticipate we will not follow it either? And perhaps something shorter? "committed" is a long word. [PUSHED]? Jason "Committed" is the accepted term in glibc. If we insist on something different, then that becomes a friction point. On the other hand, I really don't care personally as long as the meaning is clear. I'd be happy with the super-set of all of these, since it only appears in email messages, not the final commit. R.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On 03/02/2020 15:13, Richard Earnshaw (lists) wrote: On 03/02/2020 14:10, Jason Merrill wrote: On Mon, Feb 3, 2020 at 7:57 AM Alexander Monakov wrote: On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: Upper case is what glibc has, though it appears that it's a rule that is not strictly followed. If we change it, then it becomes another friction point between developer groups. Personally, I'd leave it as is, then turn a blind eye to such minor non-conformance. In that case can we simply say that both 'committed' and 'COMMITTED' are okay, if we know glibc doesn't follow that rule and anticipate we will not follow it either? And perhaps something shorter? "committed" is a long word. [PUSHED]? Jason "Committed" is the accepted term in glibc. If we insist on something different, then that becomes a friction point. "COMMITTED", not "Committed". On the other hand, I really don't care personally as long as the meaning is clear. I'd be happy with the super-set of all of these, since it only appears in email messages, not the final commit. R.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, 3 Feb 2020 15:04:57 + "Richard Earnshaw (lists)" wrote: > On 03/02/2020 14:13, Jonathan Wakely wrote: > > On Mon, 3 Feb 2020 at 14:00, Richard Earnshaw (lists) wrote: > >> Where does your '50 chars' limit come from? It's not in the glibc text, > >> and it's not in the linux kernel text either. AFAICT this is your > >> invention and you seem to be the only person proposing it. > > > > It's a fairly well established convention, e.g. > > https://chris.beams.io/posts/git-commit/ and it's what Github suggests > > (and whines if you go past it). > > > > That suggest it as a limit for everything. If you have a tag and a bug > number then then that would leave very little for the real part of the > summary and would likely lead to something meaningless or > incomprehensible in the remaining characters. That might be OK for > small projects, but for something the size of gcc, I think keeping the > extra flexibility is useful. Hopefully I've been following this discussion properly. It's important to clarify that there are two *limits* in play here. The subject should generally be kept to 50 chars or less, This is because the subject is displayed in the output of commands like 'git log --oneline' and 'git shortlog' which indents more than git log. Then the message body should wrap after 72-75 chars, again as the output of the likes of 'git log' indent the message by 4 chars. Using vim as the git commit message editor, vim will show you when your subject line is over 50 chars and it wraps the message body after 72 chars. Cheers, Andrew
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, Feb 03, 2020 at 01:59:58PM +, Richard Earnshaw (lists) wrote: > On 03/02/2020 13:54, Segher Boessenkool wrote: > >None of this are *rules*. We should not pretend they are. An email > >subject should be useful to what the receivers of that email use it for: > >see if it very interesting to them, see if it probably not interesting > >to them: that's the "smth: " at the start, and the PR number at the end, > >and of course the actual subject itself, so we should not put in too > >much fluff in the subject, there needs to be room left (in the less than > >fifty chars total) for an actual subject :-) > > > >(The example in the patch does not capitalise the subject line, btw. > >It should.) > > Where does your '50 chars' limit come from? It's not in the glibc text, > and it's not in the linux kernel text either. AFAICT this is your > invention and you seem to be the only person proposing it. Ha, no. It seems it originally is from https://github.com/git/git/commit/927a503cd07718ea0f700052043f383253904a56 and it has been part of "man git commit" since https://github.com/git/git/commit/936f32d3de468db8daf853155ddd5c6b191af60c (2006 and 2007, resp.) Segher
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, Feb 03, 2020 at 01:59:58PM +, Richard Earnshaw (lists) wrote: > On 03/02/2020 13:54, Segher Boessenkool wrote: > >None of this are *rules*. We should not pretend they are. An email > >subject should be useful to what the receivers of that email use it for: > >see if it very interesting to them, see if it probably not interesting > >to them: that's the "smth: " at the start, and the PR number at the end, > >and of course the actual subject itself, so we should not put in too > >much fluff in the subject, there needs to be room left (in the less than > >fifty chars total) for an actual subject :-) > > > >(The example in the patch does not capitalise the subject line, btw. > >It should.) > > Where does your '50 chars' limit come from? It's not in the glibc text, > and it's not in the linux kernel text either. AFAICT this is your > invention and you seem to be the only person proposing it. > > I think the linux rule (the whole line, not including the parts that are > removed on commit, should not exceed 75 characters) is far more sensible > - which is why this draft states this. Oh, and it's not a limit, just a strong recommendation. Segher
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
Hello, On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > Where does your '50 chars' limit come from? It's not in the glibc text, > and it's not in the linux kernel text either. AFAICT this is your > invention and you seem to be the only person proposing it. Nope, it's fairly common, so much so that it's included in the "commonly accepted rules" that googling for "git subject lines" gives you (as a snippet glimpse from some website), and that vim changes color when spilling over 50 chars. I actually thought it was universal and obvious until this thread (which is why I admittedly only did the above google right before writing this mail). For the rationale: 'git log --oneline' with hash and author or date should fit the usual 72 char limit. (An 11-character hash plus space alone would come out as 60 chars for the subject) That's also the reason why some people (certainly me) are nervous about or dislike all the "tags" in the subject line. E.g. what essential information (and subjects are for essential info, right?) is "[committed]" (or, even worse "[patch]") supposed to transport? If the rest of the subject doesn't interest me, I don't care if something was committed or not; if it _does_ interest me, then I'm going to look at the mail/patch either way, if committed or not; at which point the info if the author required review or has already committed it could be gives in the body as well. Similar for some other metainfo tags. (The "subsystem:" is good, though). And if we must have these tags, then why not at least short ones? Why isn't "[cmt]" or something enough? There will be very few tags, so they become mnemonic pretty much immediately. What becomes clearer when writing "[patch v2 1/13]" in comparison to "[v2 1/13]"? Ciao, Michael.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, Feb 03, 2020 at 01:59:58PM +, Richard Earnshaw (lists) wrote:
> I think the linux rule (the whole line, not including the parts that are
> removed on commit, should not exceed 75 characters) is far more sensible
> - which is why this draft states this.
FWIW, on a slightly older kernel tree:
$ git log --oneline --format=%s|awk '{print length}'|sort -n|uniq -c|sort -r
-n|head -10
25059 50
24872 49
24563 47
24547 48
24261 51
24050 52
23773 53
23527 46
23231 54
22907 45
On git.git:
$ git log --oneline --format=%s|awk '{print length}'|sort -n|uniq -c|sort -r
-n|head -10
1694 50
1657 49
1649 44
1623 46
1621 48
1606 43
1579 45
1558 47
1544 51
1499 37
Sometimes longer subject lines are unavoidable. Most of the time they
can be pretty easily avoided, and they hurt usability. It is okay to
spend a few minutes on it, think of the thousands of people who will
later read it.
Segher
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On 03/02/2020 17:31, Michael Matz wrote: Hello, On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: Where does your '50 chars' limit come from? It's not in the glibc text, and it's not in the linux kernel text either. AFAICT this is your invention and you seem to be the only person proposing it. Nope, it's fairly common, so much so that it's included in the "commonly accepted rules" that googling for "git subject lines" gives you (as a snippet glimpse from some website), and that vim changes color when spilling over 50 chars. I actually thought it was universal and obvious until this thread (which is why I admittedly only did the above google right before writing this mail). For the rationale: 'git log --oneline' with hash and author or date should fit the usual 72 char limit. (An 11-character hash plus space alone would come out as 60 chars for the subject) That's also the reason why some people (certainly me) are nervous about or dislike all the "tags" in the subject line. E.g. what essential information (and subjects are for essential info, right?) is "[committed]" (or, even worse "[patch]") supposed to transport? If the rest of the subject doesn't interest me, I don't care if something was committed or not; if it _does_ interest me, then I'm going to look at the mail/patch either way, if committed or not; at which point the info if the author required review or has already committed it could be gives in the body as well. Similar for some other metainfo tags. (The "subsystem:" is good, though). And if we must have these tags, then why not at least short ones? Why isn't "[cmt]" or something enough? There will be very few tags, so they become mnemonic pretty much immediately. What becomes clearer when writing "[patch v2 1/13]" in comparison to "[v2 1/13]"? The idea is that the [...] part is NOT part of the commit, only part of the email. 'git am' would strip leading [...] automatically unless you've configured, or asked git to do otherwise. So that leading part is not counted for the length calculation. R. Ciao, Michael.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
Hi, On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > The idea is that the [...] part is NOT part of the commit, only part of > the email. I understand that, but the subject line of this thread says "e-mail subject lines", so I thought we were talking about, well, exactly that; and I see no value of these tags in e-mails either. (They might have a low but non-zero value for projects that use a single mailing list for patches and generic discussion, but we are not such project) Basically: if they are deemed to clutter the git log for whatever reason, then there must be a very good argument for why they not also clutter e-mail subject lines, but instead are essential to have there, but not in the log. > 'git am' would strip leading [...] automatically unless > you've configured, or asked git to do otherwise. So that leading part > is not counted for the length calculation. There's still e-mail netiquette which also should be obeyed, or at least not contradicted by git netiquette. Ciao, Michael.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On 03/02/2020 17:48, Michael Matz wrote: Hi, On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: The idea is that the [...] part is NOT part of the commit, only part of the email. I understand that, but the subject line of this thread says "e-mail subject lines", so I thought we were talking about, well, exactly that; and I see no value of these tags in e-mails either. (They might have a low but non-zero value for projects that use a single mailing list for patches and generic discussion, but we are not such project) Basically: if they are deemed to clutter the git log for whatever reason, then there must be a very good argument for why they not also clutter e-mail subject lines, but instead are essential to have there, but not in the log. Well, I'd review a patch differently depending on whether or not it was already committed, a patch requiring review or an RFC looking for more general comments, so I *do* think such an email prefix is useful. 'git am' would strip leading [...] automatically unless you've configured, or asked git to do otherwise. So that leading part is not counted for the length calculation. There's still e-mail netiquette which also should be obeyed, or at least not contradicted by git netiquette. The 50 char limit seems to come from wanting git log --oneline to not wrap in an 80 column terminal. Whilst laudable, I'm not sure that such a limit doesn't become too restrictive and then lead to hard-to-understand summaries. R.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, Feb 03, 2020 at 05:48:57PM +, Michael Matz wrote: > On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > > > The idea is that the [...] part is NOT part of the commit, only part of > > the email. > > I understand that, but the subject line of this thread says "e-mail > subject lines", so I thought we were talking about, well, exactly that; > and I see no value of these tags in e-mails either. In email, they do carry information that is useful there, the distinction whether a patch has been committed already and doesn't need review from others, or whether it is a patch intended for patch review, or just a RFC patch that is not yet ready for review, but submitter is looking for some feedback. Jakub
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, 3 Feb 2020, Michael Matz wrote: > I understand that, but the subject line of this thread says "e-mail > subject lines", so I thought we were talking about, well, exactly that; > and I see no value of these tags in e-mails either. I agree that [PATCH] is not useful (and in general, anything in the subject line before the actual meaningful commit summary should be as short as possible). [RFC] might be useful, but not [PATCH]. -- Joseph S. Myers jos...@codesourcery.com
Re: Aliasing rules for unannotated SYMBOL_REFs
Thanks for the answer, and sorry for slow follow-up. Got distracted by other things... Jeff Law writes: > On Sat, 2020-01-25 at 09:31 +, Richard Sandiford wrote: >> TL;DR: if we have two bare SYMBOL_REFs X and Y, neither of which have an >> associated source-level decl and neither of which are in an anchor block: >> >> (Q1) can a valid byte access at X+C alias a valid byte access at Y+C? >> >> (Q2) can a valid byte access at X+C1 alias a valid byte access at Y+C2, >> C1 != C2? >> >> Also: >> >> (Q3) If X has a source-level decl and Y doesn't, and neither of them are >> in an anchor block, can valid accesses based on X alias valid accesses >> based on Y? > So what are the cases where Y won't have a source level decl but we > have a decl in RTL? anchors, other cases? Not really sure why I wrote "source-level" TBH. I was really talking about any symbol that has a SYMBOL_REF_DECL. I think there are three "interesting" cases: - symbols with a SYMBOL_REF_DECL - anchor symbols - bare symbols (i.e. everything else) Bare symbols are hopefully rare these days. >> (well, OK, that wasn't too short either...) > I would have thought the answer would be "no" across the board. But > the code clearly indicates otherwise. > > Interposition clearly complicates things as do explicit aliases though. > >> This part seems obvious enough. But then, apart from the special case of >> forced address alignment, we use an offset-based check even for cmp==-1: >> >> /* Assume a potential overlap for symbolic addresses that went >> through alignment adjustments (i.e., that have negative >> sizes), because we can't know how far they are from each >> other. */ >> if (maybe_lt (xsize, 0) || maybe_lt (ysize, 0)) >> return -1; >> /* If decls are different or we know by offsets that there is no >> overlap, >> we win. */ >> if (!cmp || !offset_overlap_p (c, xsize, ysize)) >> return 0; >> >> So we seem to be taking cmp==-1 to mean that although we don't know >> the relationship between the symbols, it must be the case that either >> (a) the symbols are equal (e.g. via aliasing) or (b) the accesses are >> to non-overlapping objects. In other words, one of the situations >> described by cmp==1 or cmp==0 must be true, but we don't know which >> at compile time. > Right. That was the conclusion I came to. If a SYMBOL_REF has an > alias, the alias must have the same value as the SYMBOL_REF. So their > either equal or there's no valid case for overlap. > >> >> This means that in practice, the answer to (Q1) appears to be "yes" >> but the answer to (Q2) appears to be "no". > That would be my understanding once aliases/interpositioning come into > play. > >> >> This somewhat contradicts: >> >> /* In general we assume that memory locations pointed to by different >> labels >> may overlap in undefined ways. */ >> return -1; >> >> at the end of compare_base_symbol_refs, which seems to be saying >> that the answer to (Q2) ought to be "yes" instead. Which is right? > I'm not sure how we could get to yes in that case. A symbol alias or > interposition ultimately still results in two symbols having the same > final address. Thus for a byte access if C1 != C2, then we can't have > an overlap. I think it's handling cases in which one symbol is a bare symbol (has no decl and isn't an anchor). I assumed the idea was that we could have a decl-less SYMBOL_REF for the start of a particular section, or things like that. >> In PR92294 we have a symbol X at ANCHOR+OFFSET that's preemptible. >> Under the (Q1)==yes/(Q2)==no assumption, cmp==-1 means that either >> (a) X = ANCHOR+OFFSET or (b) X and ANCHOR reference non-overlapping >> objects. So we should take the offset into account when doing: >> >> if (!cmp || !offset_overlap_p (c, xsize, ysize)) >> return 0; >> >> Let's call this FIX1. > So this is a really interesting wrinkle. Doesn't this change Q2 to a > yes? In particular it changes the "invariant" that the symbols have > the same address in the event of an symbol alias or interposition. Of > course one could ask the question of whether or not we should handle > cases with anchors specially. This wouldn't come under Q2, since that was about symbols that aren't in an anchor block. I think it just means we need to generalise the three cases that don't involve bare symbols from: - known equal - independent - equal or independent to: - known distance apart - independent - known distance apart or independent It's fortunate that anchors themselves can't be interposed. :-) >> But that then brings us to: why does memrefs_conflict_p return -1 >> when one symbol X has a decl and the other symbol Y doesn't, and neither >> of them are block symbols? Is the answer to (Q3) that we allow equality >> but not overlap here too? E.g. a linker script could define Y to X but >> not to a region that contains X at a nonzero o
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
Hello, On Mon, 3 Feb 2020, Jakub Jelinek wrote: > > > The idea is that the [...] part is NOT part of the commit, only part of > > > the email. > > > > I understand that, but the subject line of this thread says "e-mail > > subject lines", so I thought we were talking about, well, exactly that; > > and I see no value of these tags in e-mails either. > > In email, they do carry information that is useful there, the distinction > whether a patch has been committed already and doesn't need review from > others, or whether it is a patch intended for patch review, or just a RFC > patch that is not yet ready for review, but submitter is looking for some > feedback. For tags like [cmt] or [rfc] I don't have much gripe, though I do think that info could be given in the body, and that e.g. in e-mail archives (where the tags are not removed automatically) they carry the same value as in git log, namely zero. But suggesting that using the subject line for tagging is recommended can lead to subjects like [PATCH][GCC][Foo][component] Fix foo component bootstrap failure in an e-mail directed to gcc-patc...@gcc.gnu.org (from somewhen last year, where Foo/foo was an architecture; I'm really not trying to single out the author). That is, _none_ of the four tags carried any informational content. Ciao, Michael.
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
Hello, On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > Well, I'd review a patch differently depending on whether or not it was > already committed, a patch requiring review or an RFC looking for more > general comments, so I *do* think such an email prefix is useful. As I said: a very good argument must be made; it might be that rfc falls into the useful-tag category. > >> 'git am' would strip leading [...] automatically unless > >> you've configured, or asked git to do otherwise. So that leading part > >> is not counted for the length calculation. > > > > There's still e-mail netiquette which also should be obeyed, or at least > > not contradicted by git netiquette. > > The 50 char limit seems to come from wanting git log --oneline to not wrap in > an 80 column terminal. Whilst laudable, I'm not sure that such a limit > doesn't become too restrictive and then lead to hard-to-understand summaries. In my experience hard-to-understand summaries are more related to people writing them than to length, IOW, I fear a larger limit like 72 characters won't help that. And, as Segher put it, we aren't really talking about limits, only about suggestions, if you _really_ have to mention that 40-character function name in which you fixed something in your subject, then yeah, you'll go over the 50 chars. But as recommendation the 50 chars make more sense than the 72 chars, IMHO. Ciao, Michael.
Re: Git ChangeLog policy for GCC Testsuite inquiry
"H.J. Lu" writes: > On Fri, Jan 24, 2020 at 2:39 PM Paul Smith wrote: >> >> On Fri, 2020-01-24 at 22:45 +0100, Jakub Jelinek wrote: >> > > > In my experience the output of git log is a total mess so cannot >> > > > replace ChangeLogs. But we can well decide to drop ChangeLog for >> > > > the testsuite. >> > > >> > > Well, glibc has moved to extracting them from git, building >> > > policies and scripts around that. I'm pretty sure other >> > > significant projecs are also extracting their ChangeLogs from git. >> > > >> > > We could do the same, selecting some magic date as the cutover >> > > point after which future ChangeLogs are extracted from GIT. In >> > > fact, that's precisely what I'd like to see us do. >> > >> > We don't have a tool that can do it, not even get the boilerplate >> > right. Yes, mklog helps, but it very often gets stuff wrong. Not to >> > mention that the text what actually changed can't be generated very >> > easily. >> >> I don't know if it counts as a significant project, but GNU make has >> been doing this for years. >> >> What I did was take the existing ChangeLogs and rename them to >> ChangeLog.1 or whatever, then started with a new ChangeLog generated >> from scratch from Git messages. >> >> I use the gnulib build-aux/gitlog-to-changelog script to do it. It >> requires a little bit of discipline to get right; in particular you >> have to remember that the Git commit message will be indented 8 spaces >> in the ChangeLog, so you have to be careful that your commit messages >> wrap at char 70 (or less) in your Git commit. >> >> If you have Git hooks you could enforce a bit of formatting; for >> example any line not indented by space must be <=70 chars long; this >> allows people to use long lines for formatted content if they indent it >> with a space or two. >> >> Otherwise, it's the same as writing the ChangeLog and you only have to >> do it once. >> >> Just to note, the above script simply transcribes the commit message >> into ChangeLog format. It does NOT try to auto-generate ChangeLog- >> style content (files that changed, functions, etc.) from the Git diff >> or whatever. >> >> There are a few special tokens you can add to your Git commit message >> that get reformated to special changelog tokens like "(tiny change)" >> etc. >> >> As mentioned previously, it's very important that the commit message be >> provided as part of the code review, and it is very much fair game for >> review comments. This is common practice, and a good idea because bad >> commit messages are always a bummer, ChangeLog or not. >> > > Libgcrypt includes ChangeLog entries in git commit messages: > > http://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git > > In each patch, commit log starts with ChangeLog entries without leading > TABs followed by separator line with -- and then commit message. They > have a script to extract ChangeLog for release. How many people would we be catering for by generating changelogs at release time though? It seems too low-level to be useful to users, and people wanting to track gcc development history at the source level would surely be better off using git (which e.g. makes it much easier to track changes to particular pieces of code). Perhaps there are practical or policy reasons for not requiring everyone who wants to track gcc development history to build or install git. But if so, why not just include the output of "git log", with whatever options seem best? (Probably --stat at least, to show the affected files.) Like with the svn-to-git conversion, the less we change the way the history is presented, the less chance there is of something going wrong. And the idea is that git log should be informative enough for upstream developers, so surely it should be enough for others too. Thanks, Richard
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, Feb 03, 2020 at 06:54:05PM +0100, Jakub Jelinek wrote: > On Mon, Feb 03, 2020 at 05:48:57PM +, Michael Matz wrote: > > On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > > > > > The idea is that the [...] part is NOT part of the commit, only part of > > > the email. > > > > I understand that, but the subject line of this thread says "e-mail > > subject lines", so I thought we were talking about, well, exactly that; > > and I see no value of these tags in e-mails either. > > In email, they do carry information that is useful there, the distinction > whether a patch has been committed already and doesn't need review from > others, or whether it is a patch intended for patch review, or just a RFC > patch that is not yet ready for review, but submitter is looking for some > feedback. It's irrelevant whether a patch is committed or not whather it needs review, imnsho :-) "rfc" is useful, certainly. It makes clear that the sender would like some help, and/or that the subject might be controversial, both things that have more time pressure than most. Segher
Re: [PATCH, v3] wwwdocs: e-mail subject lines for contributions
On Mon, Feb 03, 2020 at 06:20:20PM +, Michael Matz wrote: > On Mon, 3 Feb 2020, Richard Earnshaw (lists) wrote: > > Well, I'd review a patch differently depending on whether or not it was > > already committed, a patch requiring review or an RFC looking for more > > general comments, so I *do* think such an email prefix is useful. > > As I said: a very good argument must be made; it might be that rfc falls > into the useful-tag category. Yes, "rfc" can be useful to know *before* reading the mail. > > The 50 char limit seems to come from wanting git log --oneline to not wrap > > in > > an 80 column terminal. Whilst laudable, I'm not sure that such a limit > > doesn't become too restrictive and then lead to hard-to-understand > > summaries. > > In my experience hard-to-understand summaries are more related to people > writing them than to length, IOW, I fear a larger limit like 72 characters > won't help that. Yup. If it helps, don't think of it as "summary", think of it as "title". Segher
Re: Git ChangeLog policy for GCC Testsuite inquiry
On Mon, 2020-02-03 at 18:55 +, Richard Sandiford wrote: > "H.J. Lu" writes: > > On Fri, Jan 24, 2020 at 2:39 PM Paul Smith wrote: > > > On Fri, 2020-01-24 at 22:45 +0100, Jakub Jelinek wrote: > > > > > > In my experience the output of git log is a total mess so cannot > > > > > > replace ChangeLogs. But we can well decide to drop ChangeLog for > > > > > > the testsuite. > > > > > > > > > > Well, glibc has moved to extracting them from git, building > > > > > policies and scripts around that. I'm pretty sure other > > > > > significant projecs are also extracting their ChangeLogs from git. > > > > > > > > > > We could do the same, selecting some magic date as the cutover > > > > > point after which future ChangeLogs are extracted from GIT. In > > > > > fact, that's precisely what I'd like to see us do. > > > > > > > > We don't have a tool that can do it, not even get the boilerplate > > > > right. Yes, mklog helps, but it very often gets stuff wrong. Not to > > > > mention that the text what actually changed can't be generated very > > > > easily. > > > > > > I don't know if it counts as a significant project, but GNU make has > > > been doing this for years. > > > > > > What I did was take the existing ChangeLogs and rename them to > > > ChangeLog.1 or whatever, then started with a new ChangeLog generated > > > from scratch from Git messages. > > > > > > I use the gnulib build-aux/gitlog-to-changelog script to do it. It > > > requires a little bit of discipline to get right; in particular you > > > have to remember that the Git commit message will be indented 8 spaces > > > in the ChangeLog, so you have to be careful that your commit messages > > > wrap at char 70 (or less) in your Git commit. > > > > > > If you have Git hooks you could enforce a bit of formatting; for > > > example any line not indented by space must be <=70 chars long; this > > > allows people to use long lines for formatted content if they indent it > > > with a space or two. > > > > > > Otherwise, it's the same as writing the ChangeLog and you only have to > > > do it once. > > > > > > Just to note, the above script simply transcribes the commit message > > > into ChangeLog format. It does NOT try to auto-generate ChangeLog- > > > style content (files that changed, functions, etc.) from the Git diff > > > or whatever. > > > > > > There are a few special tokens you can add to your Git commit message > > > that get reformated to special changelog tokens like "(tiny change)" > > > etc. > > > > > > As mentioned previously, it's very important that the commit message be > > > provided as part of the code review, and it is very much fair game for > > > review comments. This is common practice, and a good idea because bad > > > commit messages are always a bummer, ChangeLog or not. > > > > > > > Libgcrypt includes ChangeLog entries in git commit messages: > > > > http://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git > > > > In each patch, commit log starts with ChangeLog entries without leading > > TABs followed by separator line with -- and then commit message. They > > have a script to extract ChangeLog for release. > > How many people would we be catering for by generating changelogs at > release time though? It seems too low-level to be useful to users, > and people wanting to track gcc development history at the source level > would surely be better off using git (which e.g. makes it much easier to > track changes to particular pieces of code). > > Perhaps there are practical or policy reasons for not requiring everyone > who wants to track gcc development history to build or install git. > But if so, why not just include the output of "git log", with whatever > options seem best? (Probably --stat at least, to show the affected files.) > > Like with the svn-to-git conversion, the less we change the way the > history is presented, the less chance there is of something going wrong. > And the idea is that git log should be informative enough for upstream > developers, so surely it should be enough for others too. I believe the ChangeLog is primarily a FSF requirement, hence generating it from the SCM at release time seems reasonable. ANd yes, even though I have been a regular ChangeLog user, I rely more and more on the git log these days. jeff
Warning on move and dereference of unique_ptr in the same expression
Hello gcc I have now twice hit obscure bugs in Chromium that crashed on some compilers but not on others, and didn't produce any warnings on any compiler. I would like to know if this code is as undefined as I think it is, and if it would make sense to have gcc warn about it. Both cases basically has this form: std::unique_ptr a; a->b->callMethod(something, bind(callback, std::move(a))); This crashed with MSVC and gcc 5, but not with newer gcc or with clang. When it crashes it is because the arguments and the move therein have been evaluated before a->b is resolved. I assume this is undefined behavior? So why isn't the warning for using and modifying in the same expression triggered? Best regards 'Allan
Re: Warning on move and dereference of unique_ptr in the same expression
On Mon, Feb 03, 2020 at 09:26:40PM +0100, Allan Sandfeld Jensen wrote: > Hello gcc > > I have now twice hit obscure bugs in Chromium that crashed on some compilers > but not on others, and didn't produce any warnings on any compiler. I would > like to know if this code is as undefined as I think it is, and if it would > make sense to have gcc warn about it. > > Both cases basically has this form: > > std::unique_ptr a; > > a->b->callMethod(something, bind(callback, std::move(a))); > > This crashed with MSVC and gcc 5, but not with newer gcc or with clang. You mean the application itself, not the compiler, presumably. > When it crashes it is because the arguments and the move therein have been > evaluated before a->b is resolved. > > I assume this is undefined behavior? So why isn't the warning for using and > modifying in the same expression triggered? This should be defined in C++17, with P0145 in particular: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0145r3.pdf which says that the expression that names the function is sequenced before every argument expression and every default argument. Marek
Re: Warning on move and dereference of unique_ptr in the same expression
On Montag, 3. Februar 2020 21:47:13 CET Marek Polacek wrote: > On Mon, Feb 03, 2020 at 09:26:40PM +0100, Allan Sandfeld Jensen wrote: > > Hello gcc > > > > I have now twice hit obscure bugs in Chromium that crashed on some > > compilers but not on others, and didn't produce any warnings on any > > compiler. I would like to know if this code is as undefined as I think it > > is, and if it would make sense to have gcc warn about it. > > > > Both cases basically has this form: > > > > std::unique_ptr a; > > > > a->b->callMethod(something, bind(callback, std::move(a))); > > > > This crashed with MSVC and gcc 5, but not with newer gcc or with clang. > > You mean the application itself, not the compiler, presumably. Of course. > > > When it crashes it is because the arguments and the move therein have been > > evaluated before a->b is resolved. > > > > I assume this is undefined behavior? So why isn't the warning for using > > and > > modifying in the same expression triggered? > > This should be defined in C++17, with P0145 in particular: > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0145r3.pdf > which says that the expression that names the function is sequenced before > every argument expression and every default argument. > Right thanks, that would explain why it worked consistently with the latest gcc versions. I guess it is more of a corner case to ask for it being warned about in --std=c++14 mode? 'Allan
