exportloopref icon indicating copy to clipboard operation
exportloopref copied to clipboard

False positive when a loop var is exported, directly followed by break

Open alvaroaleman opened this issue 3 years ago • 4 comments

exportloopref complains about something like this, even though it is valid:

var result *int
haystack := []int{1, 2}
for _, hay := range haystack {
 if true {
   result = &hay
   break
  }
}

alvaroaleman avatar Jul 16 '21 23:07 alvaroaleman

:memo: to generalize

:thought_balloon: What kind of structure is a alignment directly followed by a break?

result = &hay
(...something no mattered...)
break

NOTE: no mattered equals no condition, no continue.

-> there's no problem.

no problem cases

directly followed by break:

result = &p
break-

there's steps that never continue:

result = &p
print(result)
break

breaks on parent branch:

if <cond.1> {
    if <cond.2> {
        result = &p
    }
    break
}

diagnostics

breaks sometimes

result = &p
if <cond> {
    break
}
result = &p
...
if <cond> {
    continue // it will ignore the breaking branch.
}
...
break

Cost to probe the diagnostics

  • Easy to probe
    • Which loop will be broken.
  • Hard to probe (high cost)
    • There's break after the alignment.
    • There's no condition (if) between the alignment and break.
    • There's no continue between the alignment and break.

How to do

  • First pass, find the exporting alignments as candidates.
  • Second pass, check whether they are followed by break or not.
  • Third pass, check whether continue exists before break.

Worry

Maybe to resolve this, exportloopref will be slow (about 3 times) Even if we care just about the DIRECTLY followed by break (without any lines between the break and alignment), it needs 2 times cost.

kyoh86 avatar Jul 31 '21 03:07 kyoh86

To make it simple:

First pass

  • Store the candidates with if, for and switch stacks
  • Store breaks with if, for and switch stacks.
  • Store continues with parent loop.

Second pass

For each candidates:

  • Check whether there's any break which has same stack.
  • Check whether the loop has no continue.

kyoh86 avatar Jul 31 '21 05:07 kyoh86

@kyoh86 thank you for looking into this! I would already be very happy if the case of "pointer assignment directly followed by break/return" would work.

alvaroaleman avatar Aug 02 '21 13:08 alvaroaleman

Your requirement is so simple. But I have not found a simple way to just do it. I'm trying to look for it but also to implement complex pass that is, as shown above.

If you have an idea, please give me a pull request.

kyoh86 avatar Aug 02 '21 17:08 kyoh86