paulk-asert commented on PR #3082:
URL: https://github.com/apache/groovy/pull/3082#issuecomment-6564277121
@jdaugherty thanks, this looks good to me, and your reasoning on the type
check makes sense: the class's own logger field is always added before
`addGuard` runs, and re-scoping binds the reference to it afterwards.
Could you add a test that pins that reasoning? The current test has `@Log`
on both classes, so the field types match. A superclass with a private `log`
field of another type covers the "whatever its type" argument and the
mixed-strategy case together, without needing a second logging library:
```groovy
@Test
void testLogGuardWithNonLoggerPrivateFieldInSuperclass() {
def texts = compileBothWays([
'p/Base.groovy': 'package p; abstract class Base { private static
final String log = "not a logger" }',
'p/Sub.groovy': 'package p; @groovy.util.logging.Log class Sub
extends Base { void run(Object o) { log.info("value: " + o) } }'
], ['p/Sub.groovy'])
def (together, separate) = texts['p.Sub']
assert together == separate
assert separate.contains('IFEQ')
}
```
If the guard ever bound to the superclass's field, this would fail, since
`String` has no `isLoggable`.
I also checked a superclass `log` *property* (`def log`, `static log`,
`String log`): with your change the guard is added in both compilations and the
call uses `Sub.log`, since the reference binds to the property's private
backing field. Not needed as a test, but it could go in the same parameterized
test if you like.
Interestingly, on master the guard is also dropped when the superclass
declares its own `log` and both classes are compiled *together*, so the fix is
a bit wider than the incremental-build case in the description.
A minor, non-blocking point: yes please to widening the javadoc on
`refersToLogField` as you offered. It currently says the superclass is "read
from bytecode" and its field is "not accessible here", but the same binding
happens for a private `log` declared in a superclass compiled in the same run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
