> 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. >> > > It is correct that the range loop variable can be used in a goroutine > safely. However, the existing loopclosure vet check will not report on this > instance. >
VetBot uses a variant of loopclosure which I modified to be more sensitive (to avoid false-negatives). The variant in VetBot does report code like this example <https://github.com/github-vet/bots/commit/c83190e474614a918cdfc426240dc4034df54d4c> even though it is a false-positive, due to the break. > When evaluating the change, we will need to find cases where the semantics > of the program might differ after the change. Semantic changes include race > conditions being eliminated (like in the Bugs > <https://github.com/github-vet/rangeloop-pointer-findings#bugs> section > for the project). It can also include changes to the memory layout after > the loop (one aliased memory allocation vs a memory allocation per > iteration being stored in memory),changes in branching behavior (pointer > comparisons), changes in variable lifetime when an escape does happen > (which can impact when finalizers get run), and order of calls to > external functions. Looking at closures of loop variables and escaping > references is definitely a good start. I am not yet sure exactly what would > suffice (but happy to chat over email). I like how this list starts by laying out all the semantic changes in the proposal rather than starting from asking 'what can go wrong' -- which is where I started. Now that we have this list -- I'm looking at each of the entries in this list and asking myself "what could go wrong, and how can we detect it?" There are a few cases where I'm not able to come up with a satisfactory answer on my own. I'm hoping that by laying out my understanding below in a transparent way, someone might be willing to help fill in the gaps. race conditions being eliminated (like in the Bugs > <https://github.com/github-vet/rangeloop-pointer-findings#bugs> section > for the project) > It's almost certainly the case that any current code which relies on the existence of a race condition for its correct operation is flaky. We can probably safely classify these as 'beneficial' semantic changes, but that can't be a unilateral decision. If we're willing to accept that any code that passes a range-loop pointer to a goroutine is going to have its behavior improved by the change, we can avoid many of the issues being reported by the prototype. changes to the memory layout after the loop (one aliased memory allocation > vs a memory allocation per iteration being stored in memory) > It seems to me that this can only break existing code which relies on the very specific way in which variables are laid out in memory. Based on the little I know, I would expect this to require the code to do something with the unsafe package or use CGO. Are there other ways to make code rely on memory layout in Go (other than things like pointer comparisons and finalizers mentioned below)? changes in branching behavior (pointer comparisons) > This makes sense and it seems straightforward to check for by finding instances where a range-loop reference is used in a boolean expression. changes in variable lifetime when an escape does happen (which can impact > when finalizers get run) I'm still processing this one; but after a glance, it looks like we *might *only be interested in when a range-loop reference is passed as the first argument to `runtime.SetFinalizer`. If that's the case, writing an analyzer should be straightforward. order of calls to external functions I must be missing something here, since I am not sure how the proposed language change can impact the order in which calls to external functions are made. I don't think that I'm grasping your intended meaning. -- 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CALJzkY-ev_eDunVCuB4wzL499B0heE%3DhuCWbc7zyN7GQPPhTWg%40mail.gmail.com.
