Copilot commented on code in PR #13737:
URL: https://github.com/apache/skywalking/pull/13737#discussion_r2912527476
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALBlockCodegen.java:
##########
@@ -1140,6 +1172,288 @@ static String generateExtraLogAccess(
return prevVar;
}
+ // ==================== Def statement codegen ====================
+
+ static void generateDefStatement(final StringBuilder sb,
+ final LALScriptModel.DefStatement def,
+ final LALClassGenerator.GenCtx genCtx) {
+ final LALScriptModel.ValueAccess init = def.getInitializer();
+ final String varName = def.getVarName();
+ final String javaVar = "_" + varName;
Review Comment:
`def` variables are mapped to Java locals using `"_" + varName` (e.g., `def
p` -> `_p`). This can collide with compiler-reserved locals like `_p` (extraLog
cast), `_o` (output builder), `_tN` (proto cache), etc., leading to
uncompilable generated code when users pick those names. Consider using a
dedicated prefix for def vars (e.g., `__d_`/`_def_`) and/or detecting/resolving
collisions against reserved names before emitting declarations.
```suggestion
final String javaVar = "__d_" + varName;
```
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALBlockCodegen.java:
##########
@@ -1174,29 +1488,29 @@ static void generateProcessRegistryCall(
// ==================== Utility methods ====================
static String appendMethodSegment(final String current,
- final LALScriptModel.MethodSegment ms) {
+ final LALScriptModel.MethodSegment ms,
+ final LALClassGenerator.GenCtx genCtx) {
+ final String mn = ms.getName();
+ final String args = ms.getArguments().isEmpty()
+ ? "" : generateMethodArgs(ms.getArguments(), genCtx);
if (ms.isSafeNav()) {
- final String mn = ms.getName();
+ // Special-cased helpers for common safe-nav methods on Object
if ("toString".equals(mn)) {
return "h.toString(" + current + ")";
} else if ("trim".equals(mn)) {
return "h.trim(" + current + ")";
- } else {
- throw new IllegalArgumentException(
- "Unsupported safe-nav method: ?." + mn + "()");
}
+ // General safe-nav: null guard with ternary
+ return "(" + current + " == null ? null : "
+ + current + "." + mn + "(" + args + "))";
} else {
Review Comment:
`appendMethodSegment` emits a generic safe-nav ternary for any method:
`(current == null ? null : current.method(...))`. If the method returns a
primitive (e.g., `String?.length()` -> `int`), this generates invalid Java due
to mixing `null` with a primitive in the ternary. Either restrict safe-nav
methods here (as previously) or add type-aware handling/boxing for primitive
return types when generating generic safe-nav method calls.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALBlockCodegen.java:
##########
@@ -1209,6 +1523,19 @@ static String generateMethodArgs(
va.getSegments().get(0))).append("\"");
} else if (va.isNumberLiteral()) {
sb.append(va.getSegments().get(0));
+ } else if (!va.getSegments().isEmpty()) {
+ final String text = va.getSegments().get(0);
+ if ("true".equals(text) || "false".equals(text)
+ || "null".equals(text)) {
+ // Boolean or null literal
+ sb.append(text);
+ } else if (genCtx != null
+ && genCtx.localVars.containsKey(text)) {
+ // Local def variable reference
+ sb.append(genCtx.localVars.get(text).javaVarName);
+ } else {
+ sb.append("null");
Review Comment:
`generateMethodArgs` silently converts any non-literal identifier argument
that isn’t a known `def` variable into `null`. With `def` vars now supported as
method args, this makes typos or forward references (using a variable before
it’s declared) compile but behave incorrectly at runtime. Consider failing
compilation with a clear error when an identifier arg can’t be resolved to a
local `def` variable (instead of emitting `null`).
```suggestion
throw new IllegalStateException(
"Unknown identifier used as method argument: '" +
text + '\''
);
```
--
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]
