Good point, and thanks for the response.

This example avoids the race condition by breaking out of the loop
immediately afterward. IIRC, loopclosure also reports in case the range
loop variable is passed as an argument using a name that shadows the loop
variable.

I think any loopclosure findings which work properly like this do have to
do so because they mitigate the race condition like these examples do. I
don't think examples like this can be the kind we are (really) looking for:
an instance where the proposed change
<https://github.com/golang/go/issues/20733> would break code that behaves
correctly today.

On Wed, Dec 30, 2020, 10:03 AM jake...@gmail.com <jake6...@gmail.com> wrote:

>
> On Tuesday, December 29, 2020 at 1:21:04 PM UTC-5 k.alex...@gmail.com
> wrote:
>
>> I'd like to solicit some input from the community regarding a decision
>> that could reduce the effort required in crowdsourcing.
>>
>> After thinking carefully about the loopclosure vet check
>> <https://github.com/golang/tools/blob/master/go/analysis/passes/loopclosure/loopclosure.go>,
>> it seems to me that any time it reports an issue, there actually is an
>> issue. That has also been the case for everything I've seen GitHub Vet
>> report from loopclosure thus far.
>>
>> What loopclosure does is find instances where a range loop variable is
>> used inside a goroutine or defer statement. My understanding is that, in
>> every case, this raises the potential for a race-condition between updating
>> the range loop variable and reading from it within the body of the
>> goroutine. So unless I'm missing something, it seems to me that there is
>> not any need for a human to double-check loopclosure results.
>>
>
> Technically, I believe a range loop variable can be used in a goroutine
> safely. A very contrived example: https://play.golang.org/p/jgZbGg_XP6S .
> In practice I can not see much use for this kind of pattern, but it does
> exist.
>
>
>>
>> So my plan -- unless someone comes up with something I have missed -- is
>> to omit loopclosure findings from the crowdsourcing effort. Once this
>> change is made, loopclosure findings will be kept on disk, but not uploaded
>> to GitHub for crowdsourcing.
>>
>> It's worth mentioning that there are issues which loopclosure misses
>> which VetBot is able to find, and the community's effort will still be
>> needed to help sift through false-positives once the project has stabilized.
>>
>>
>> On Sun, Dec 20, 2020 at 12:08 PM K. Alex Mills <k.alex...@gmail.com>
>> wrote:
>>
>>> Hello Gophers!
>>>
>>> During a Q&A session at this year's GopherCon, some members of the Go
>>> language team indicated a willingness to modify the behavior of range
>>> loops <https://github.com/golang/go/issues/20733> if we can be
>>> reasonably certain that the change will not cause incorrect behavior in
>>> current programs. To make that determination, a large collection of
>>> real-world go programs would need to be vetted. If we can find that, in
>>> every case, modifying the compiler behavior does not lead to an undesirable
>>> outcome, we will have strong evidence that the change may only have a small
>>> impact.
>>>
>>> I've been working on a project to gather and crowdsource data for that
>>> sort of analysis. It has reached a point where I think that it's time to
>>> share it with the Go community.
>>>
>>> The GitHub Vet Project <https://github.com/github-vet> performs static
>>> analysis on public Go repositories hosted on GitHub in order to better
>>> understand the impact of this proposed language change
>>> <https://github.com/golang/go/issues/20733>. It consists of two bots.
>>> VetBot crawls GitHub to snippets of code it thinks could be interesting, it
>>> reports it as an issue in a separate repository
>>> <https://github.com/github-vet/rangeloop-pointer-findings>, for humans
>>> to analyze via crowdsourcing. TrackBot
>>> <https://github.com/github-vet/bots/tree/main/cmd/track-bot> manages
>>> the crowdsourcing workflow.
>>>
>>> At this point, all of the features I think are necessary for the
>>> project's success have been implemented. But I am only one Gopher, and so I
>>> would like to a) make the community aware of the project and b) ask the
>>> community to help review it in three ways:
>>>
>>> 1) Test out TrackBot and the project instructions by actually
>>> crowdsourcing through the issues found so far
>>> <https://github.com/github-vet/rangeloop-pointer-findings>.
>>> 2) Review the output
>>> <https://github.com/github-vet/rangeloop-pointer-findings/issues> and
>>> raise concern if anything that I claim ought to be excluded
>>> <https://github.com/github-vet/bots/tree/main/cmd/vet-bot#2-run-static-analysis>
>>> is somehow making it through.
>>> 3) Static analysis experts: review the analyzers
>>> <https://github.com/github-vet/bots/tree/main/cmd/vet-bot> and
>>> ruthlessly question anything that could lead to a false negative. I don't
>>> think anything exists, but one pair of eyes is often wrong, and there are
>>> many folks on this list with (much) more expertise than me.
>>>
>>> One final thing. The crowd-sourcing workflow I've designed relies on the
>>> opinion of experts to help estimate the reliability of the community
>>> assessment. In order for that to work, I need the help of some Go experts
>>> who are willing to commit some time to reviewing the findings. If you'd
>>> like to participate in this way, first, read the TrackBot documentation
>>> <https://github.com/github-vet/bots/tree/main/cmd/track-bot> to
>>> understand what being an expert entails. If you're still interested, email
>>> me with the subject line 'GitHub Vet Expert' and include your GitHub
>>> username and a brief outline of your experience with Go.
>>>
>>> It's my hope that this project can provide some data that can help to
>>> move Go forward. To that end, I'm also interested in any and all feedback
>>> and suggestions. Contributions are also welcome
>>> <https://github.com/github-vet/bots/issues>.
>>>
>>> Thanks for reading,
>>>
>>> K. Alex Mills, Ph.D
>>>
>> --
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-nuts+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/golang-nuts/72281283-6824-4396-abd2-0dcb2b08ad62n%40googlegroups.com
> <https://groups.google.com/d/msgid/golang-nuts/72281283-6824-4396-abd2-0dcb2b08ad62n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/CALJzkY-XeLR3qU%3DnW3s3AwXY_GeLSAp8LXHW2scgYKatkLo4qQ%40mail.gmail.com.

Reply via email to