object.d includes this function _postblitRecurse that's intentionally public but undocumented. As far as I can see only unittests are using it. What's the deal with it? Thanks! -- Andrei

On Friday, March 02, 2018 13:32:24 Andrei Alexandrescu via Digitalmars-d wrote:
> object.d includes this function _postblitRecurse that's intentionally public but undocumented. As far as I can see only unittests are using it. What's the deal with it? Thanks! -- Andrei

It looks like it was trying to workaround a compiler bug with regards to attribute inference and destruction with the idea that it would be used in emplace, but nothing outside of object.d's unit tests use it right now. It looks like all of the changes in Phobos that used may have gotten removed due to regressions that it caused, but _postblitRecurse itself wasn't removed. And it looks like the issue in the compiler may have been fixed, making _postblitRecurse redundant, but all I know is what I can see in git log and a quick glance through the related PR on github. I'd have to read through it all in more detail to accurately assess what happened, though it's clear that it's not used now.

https://github.com/dlang/druntime/pull/1181

BTW, in case you didn't know, you can use git blame to figure out which commit last changed a set of lines, and then you can search for that commit on github to find the PR that it relates to, which is how I found this info.

- Jonathan M Davis

On 03/02/2018 01:50 PM, Jonathan M Davis wrote:
> On Friday, March 02, 2018 13:32:24 Andrei Alexandrescu via Digitalmars-d
> wrote:
>> object.d includes this function _postblitRecurse that's intentionally
>> public but undocumented. As far as I can see only unittests are using
>> it. What's the deal with it? Thanks! -- Andrei
> 
> It looks like it was trying to workaround a compiler bug with regards to
> attribute inference and destruction with the idea that it would be used in
> emplace, but nothing outside of object.d's unit tests use it right now. It
> looks like all of the changes in Phobos that used may have gotten removed
> due to regressions that it caused, but _postblitRecurse itself wasn't
> removed. And it looks like the issue in the compiler may have been fixed,
> making _postblitRecurse redundant, but all I know is what I can see in git
> log and a quick glance through the related PR on github. I'd have to read
> through it all in more detail to accurately assess what happened, though
> it's clear that it's not used now.
> 
> https://github.com/dlang/druntime/pull/1181
> 
> BTW, in case you didn't know, you can use git blame to figure out which
> commit last changed a set of lines, and then you can search for that commit
> on github to find the PR that it relates to, which is how I found this info.

Yah, it blames to Jakob Ovrum. But I don't have a simple method to ascribe the blame to a specific PR. Is the only way to look at the date then look at the log? Thanks. -- Andrei

On Fri, Mar 02, 2018 at 02:23:14PM -0500, Andrei Alexandrescu via Digitalmars-d wrote: [...]
> But I don't have a simple method to ascribe the blame to a specific PR. Is the only way to look at the date then look at the log? Thanks. -- Andrei

This is how I usually do it:

1) Find the hash of the offending git commit, usually using git bisect
   or git blame.

2) git checkout master; git log --graph | less
   (In place of `less`, use any pager that has scrollback capability. Or
   leave out the `| less` if the default pager invoked by git has
   scrollback ability.)

3) Do a search for the commit's hash. Usually, this will be a commit
   buried somewhere inside a series of commits in the offending PR.

4) Scroll up until you find the commit that merged the PR. The commit
   message will contain the PR number.  Note that this is why you need
   --graph in the call to git log, because otherwise commits from random
   PRs will be interspersed together and there will be no easy way to
   trace them.  Using --graph will give you a visual cue as to which
   commits belong to the PR so that you can trace it to the merging
   point, and it also does a topological sort, which tends to cluster
   related commits together better than the default setting.

There's probably a one-liner to do this, but I prefer using git log --graph because it also lets me see the context of the original commit and scroll around to see any other commits that might be relevant.


T

-- 
People walk. Computers run.

On 3/2/18 2:45 PM, H. S. Teoh wrote:
> On Fri, Mar 02, 2018 at 02:23:14PM -0500, Andrei Alexandrescu via Digitalmars-d wrote:
> [...]
>> But I don't have a simple method to ascribe the blame to a specific
>> PR. Is the only way to look at the date then look at the log? Thanks.
>> -- Andrei
> 
> This is how I usually do it:
> 
> 1) Find the hash of the offending git commit, usually using git bisect
>     or git blame.
> 
> 2) git checkout master; git log --graph | less
>     (In place of `less`, use any pager that has scrollback capability. Or
>     leave out the `| less` if the default pager invoked by git has
>     scrollback ability.)
> 
> 3) Do a search for the commit's hash. Usually, this will be a commit
>     buried somewhere inside a series of commits in the offending PR.
> 
> 4) Scroll up until you find the commit that merged the PR. The commit
>     message will contain the PR number.  Note that this is why you need
>     --graph in the call to git log, because otherwise commits from random
>     PRs will be interspersed together and there will be no easy way to
>     trace them.  Using --graph will give you a visual cue as to which
>     commits belong to the PR so that you can trace it to the merging
>     point, and it also does a topological sort, which tends to cluster
>     related commits together better than the default setting.
> 
> There's probably a one-liner to do this, but I prefer using git log
> --graph because it also lets me see the context of the original commit
> and scroll around to see any other commits that might be relevant.
> 

It's easier than that.

The git blame view in github shows it as this commit:

https://github.com/dlang/druntime/commit/f98a02142767d2d14b574cd381670dbd53b90d36

If you look in the upper left, it shows "master (#1181)", where 1181 is the PR that merged it. Click on the "#1181" and it brings you to the PR.

-Steve

On 3/2/18 3:02 PM, Steven Schveighoffer wrote:

> It's easier than that.
> 
> The git blame view in github shows it as this commit:
> 
> https://github.com/dlang/druntime/commit/f98a02142767d2d14b574cd381670dbd53b90d36 
> 
> 
> If you look in the upper left, it shows "master (#1181)", where 1181 is the PR that merged it. Click on the "#1181" and it brings you to the PR.

My full workflow for tracing the history of some code (in case anyone is interested):

1. Go to the file.
2. Click on the line number of the line that I'm interested in.
3. Scroll back to the top, click on blame. It will switch to blame mode and auto-scroll back to the line I clicked on earlier.
4. Click on the commit that made the line happen.

At that point, I can get the PR, as shown above. Or in many cases, the line is moved from somewhere else. Then I do this procedure:

1. Click on "parent commit"
2. Click on "browse Files" button at the top.
3. Find the location of the old line.
4. repeat phase 1.

-Steve

On Friday, March 02, 2018 14:23:14 Andrei Alexandrescu via Digitalmars-d wrote:
> On 03/02/2018 01:50 PM, Jonathan M Davis wrote:
> > On Friday, March 02, 2018 13:32:24 Andrei Alexandrescu via Digitalmars-d
> >
> > wrote:
> >> object.d includes this function _postblitRecurse that's intentionally public but undocumented. As far as I can see only unittests are using it. What's the deal with it? Thanks! -- Andrei
> >
> > It looks like it was trying to workaround a compiler bug with regards to attribute inference and destruction with the idea that it would be used in emplace, but nothing outside of object.d's unit tests use it right now. It looks like all of the changes in Phobos that used may have gotten removed due to regressions that it caused, but _postblitRecurse itself wasn't removed. And it looks like the issue in the compiler may have been fixed, making _postblitRecurse redundant, but all I know is what I can see in git log and a quick glance through the related PR on github. I'd have to read through it all in more detail to accurately assess what happened, though it's clear that it's not used now.
> >
> > https://github.com/dlang/druntime/pull/1181
> >
> > BTW, in case you didn't know, you can use git blame to figure out which commit last changed a set of lines, and then you can search for that commit on github to find the PR that it relates to, which is how I found this info.
> Yah, it blames to Jakob Ovrum. But I don't have a simple method to ascribe the blame to a specific PR. Is the only way to look at the date then look at the log? Thanks. -- Andrei

I just searched for the commit on github, and it lists both the commit and the PR on the left (though it's easy to miss, because you have to click on those boxes on the left, and on the right, it claims that the search was empty, because it didn't find any code that had that text in it). And of course, as Steven pointed out here, if you do the blame on github, you can find it that way.

- Jonathan M Davis