It's a bit more subtle:

 global a probe foo { a++ }    # we'd like a printed at end
 global b probe bar { if (b++ > 5) ... }   # but not b

It's not exactly the ++ operation that's the discriminator
between these two cases, but whether the result is used
an rvalue or whether it's tossed away.

Created attachment 8066 [details]
Possible patch

It seems like the issue is that the varuse_collecting_visitor treats the following two as equivalent:

  a++ ...
  if (a++) ...

I.e. when 'a++' is visited, it doesn't matter whether it's part of an if statement or not. In both cases, the 'a' referent is simply added to the 'read' and 'write' sets but NOT to the 'used' set, causing it to get printed. (The 'used' set contains the vars that are read while not in a rmw context -- this distinction is necessary because a rmw operation is both a read and a write, so the rule for global var printing can't just be "all vars written to but never read" otherwise expr_statements like 'a++' would not trigger auto printing).

Note BTW that this is also an issue with for/while loop conditionals.

The following patch provides a potential fix for this issue. It overrides the if/for visit methods so that their conditionals are visited as if they were lvalues (and thus cause the symbols to be added to the 'used' set). This technique is already used for visit_print_format() and visit_delete_statement() (see also commit d20a83f8).

I have yet to do a full test run with the patch above, but at the very least global_end.exp passes. Also thinking about how to clean up this part of the code to be more explicit.

On saving last_lvalue_read - it should be a complete surprise to enter visit_if_statement with current_lvalue_read=true in the first place.  An if_statement never has a value, so no-one should be indicating that they want to read it!  In fact, it ought to be initially false for all statement types, and perhaps this should be asserted.

Anyway, some additional expressions which should always be considered used:
- functioncall->args
- foreach_loop->array_slice
- foreach_loop->limit
- return_statement->value
- ternary_expression->cond
- entry_op->operand (unsure, may be translated away already)

Created attachment 8078 [details]
new patch

Hey Josh,

Thanks for all the extra cases to check. Just also noticed now that there is an issue with current_lvalue_read and functioncalls.

global a

function foo() {
   a++
   return "foo"
}

probe oneshot {
   println(foo())
}

The script above interprets the a++ in foo() as used because visit_print_format sets current_lvalue_read to true.

The following patch fixes the above as well as all the cases mentioned so far.

Here is my testing script:

global a // if condition
global b // function arg

global arr // used in foreach
global c // foreach array slice
global d // foreach limit

global e // return value
global f // ternary cond
global g // while condition

function foo(b) {
   if (b)
      println("b")
}

function bar() {
   return ++e;
}

probe oneshot {

   if (++a)
      println("a")

   foo(++b)

   arr[1,1] = 1
   foreach ([i,j] in arr[*,++c] limit ++d) {
      println("c")
      println("d")
   }

   if (bar()) {
      println("e")
   }

   if (++f ? 1 : 0) // can only be tested without the patch for if_statement applied
      println("f")

   while (!g++)
      println("g")
}

Without patch applied:

a
b
c
d
e
f
g
a=0x1
b=0x1
c=0x1
d=0x1
e=0x1
f=0x1
g=0x2

With patch applied:

a
b
c
d
e
f
g

(Not being printed automatically indicates that it was added to the used set). I will doublecheck that all spots have been covered and run it through the tester.

Created attachment 8079 [details]
new patch

Let me try that again as a diff for bugzilla.

Is the complementary case already well-covered by tests, ie
those where a++ etc. exist in various contexts that are not
meant to trigger the 'do-not-print-because-it's-used'
heuristic?

(In reply to Frank Ch. Eigler from comment #6)
> Is the complementary case already well-covered by tests, ie
> those where a++ etc. exist in various contexts that are not
> meant to trigger the 'do-not-print-because-it's-used'
> heuristic?

It's probably sort of tested indirectly, but I will make a testcase testing both sides explicitly.

Also found the same issue with arrayindex->indexes.

Fixed in commit f24849b.
Added subtest to global_end.exp in commit 7077306.