Copilot commented on code in PR #13858:
URL: https://github.com/apache/skywalking/pull/13858#discussion_r3178217966
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java:
##########
@@ -865,80 +1218,443 @@ static void generateProcessRegistryCall(
// ==================== Utility methods ====================
+ // ==================== Binary expression codegen ====================
+
+ /**
+ * JLS-style numeric type tag used for binary numeric promotion. Ordered
+ * narrow → wide, so {@code commonType(a, b)} can return the wider of two.
+ */
+ private enum ExprType {
+ INT, LONG, FLOAT, DOUBLE,
+ STRING, BOOLEAN, OBJECT, UNKNOWN;
+
+ boolean isNumeric() {
+ return this == INT || this == LONG || this == FLOAT || this ==
DOUBLE;
+ }
+ }
+
+ /**
+ * Parsed representation of a NUMBER literal token. Carries the inferred
+ * Java type and the literal text trimmed/suffixed for direct emission as
+ * Java source.
+ */
+ private static final class NumericLiteral {
+ final ExprType type;
+ /** Java-source representation, e.g. "10000", "10000L", "1.5", "1.5f".
*/
+ final String javaText;
+
+ private NumericLiteral(final ExprType type, final String javaText) {
+ this.type = type;
+ this.javaText = javaText;
+ }
+
+ static NumericLiteral parse(final String numText) {
+ String t = numText;
+ char suffix = 0;
+ if (!t.isEmpty()) {
+ final char last = t.charAt(t.length() - 1);
+ if (last == 'L' || last == 'l' || last == 'F' || last == 'f'
+ || last == 'D' || last == 'd') {
+ suffix = last;
+ t = t.substring(0, t.length() - 1);
+ }
+ }
+ final boolean fractional = t.contains(".") || t.contains("e") ||
t.contains("E");
+ switch (suffix) {
+ case 'L':
+ case 'l':
+ return new NumericLiteral(ExprType.LONG, t + "L");
+ case 'F':
+ case 'f':
+ return new NumericLiteral(ExprType.FLOAT,
+ (fractional ? t : (t + ".0")) + "f");
+ case 'D':
+ case 'd':
+ return new NumericLiteral(ExprType.DOUBLE,
+ (fractional ? t : (t + ".0")) + "d");
+ default:
+ if (fractional) {
+ return new NumericLiteral(ExprType.DOUBLE, t);
+ }
+ // Bare integer literal — INT if it fits, else LONG.
+ try {
+ Integer.parseInt(t);
+ return new NumericLiteral(ExprType.INT, t);
+ } catch (NumberFormatException e) {
+ return new NumericLiteral(ExprType.LONG, t + "L");
Review Comment:
Any integer that does not fit in `int` is treated as `long` here, but there
is no check that it actually fits in Java's `long` range. Literals like
`999999999999999999999999` will be emitted as an invalid `...L` token and fail
later during source compilation with an opaque error instead of a clear parser
diagnostic.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALScriptParser.java:
##########
@@ -684,8 +747,8 @@ private static ValueAccess visitValueAccessTerm(
chain.add(new LALScriptModel.MethodSegment(
fi.functionName().getText(), visitFunctionArgs(fi), true));
} else if (seg instanceof LALParser.SegmentIndexContext) {
- final int index = Integer.parseInt(
- ((LALParser.SegmentIndexContext) seg).NUMBER().getText());
+ final int index = (int) parseStrictInteger(
+ ((LALParser.SegmentIndexContext) seg).NUMBER().getText(),
"[index]");
Review Comment:
Casting the parsed index straight to `int` can silently wrap large literals
instead of rejecting them. For example, `[3000000000]` now becomes a negative
index, whereas the old `Integer.parseInt(...)` path failed fast. Please
range-check before narrowing so oversized indexes still produce a compile-time
error.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALScriptParser.java:
##########
@@ -736,14 +799,71 @@ private static List<LALScriptModel.FunctionArg>
visitFunctionArgs(
}
private static String resolveValueAsString(final
LALParser.ValueAccessContext ctx) {
- final LALParser.ValueAccessPrimaryContext primary =
- ctx.valueAccessTerm(0).valueAccessPrimary();
+ final LALParser.ValueAccessTermContext term = singleTermOf(ctx);
+ if (term == null) {
+ return ctx.getText();
+ }
+ final LALParser.ValueAccessPrimaryContext primary =
term.valueAccessPrimary();
if (primary instanceof LALParser.ValueStringContext) {
return stripQuotes(((LALParser.ValueStringContext)
primary).STRING().getText());
}
return primary.getText();
}
+ /**
+ * Returns the single {@link LALParser.ValueAccessTermContext} of a
+ * {@code valueAccess} when it has no arithmetic operators, otherwise null.
+ */
+ private static LALParser.ValueAccessTermContext singleTermOf(
+ final LALParser.ValueAccessContext ctx) {
+ final LALParser.ValueAccessAddContext add = ctx.valueAccessAdd();
+ if (add.valueAccessMul().size() != 1) {
+ return null;
+ }
+ final LALParser.ValueAccessMulContext mul = add.valueAccessMul(0);
+ if (mul.valueAccessTerm().size() != 1) {
+ return null;
+ }
+ return mul.valueAccessTerm(0);
+ }
+
+ /**
+ * Parse a NUMBER literal that must represent a plain integer (no
+ * decimal, no exponent, no Java-style suffix). Used by integer-only
+ * grammar slots — {@code rateLimit { rpm N }} and {@code list[N]} —
+ * which share the lexer NUMBER token with arithmetic expressions but
+ * cannot accept the suffixes/forms supported there. Throws a clear
+ * compile-time error instead of letting NumberFormatException leak
+ * later.
+ */
+ private static long parseStrictInteger(final String numText, final String
slot) {
+ for (int i = 0; i < numText.length(); i++) {
+ final char c = numText.charAt(i);
+ if (c < '0' || c > '9') {
+ throw new IllegalArgumentException(
+ slot + " expects a plain integer literal, got '" + numText
+ + "' (suffixes / decimals / exponents are not accepted
here)");
+ }
+ }
+ return Long.parseLong(numText);
Review Comment:
`parseStrictInteger` still lets `Long.parseLong(...)` throw on overflow, so
extremely large `rpm` / index literals now leak a raw `NumberFormatException`
instead of the clear `IllegalArgumentException` this helper promises. Please
catch that overflow case and rethrow a parser-level error with the slot name.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java:
##########
@@ -865,80 +1218,443 @@ static void generateProcessRegistryCall(
// ==================== Utility methods ====================
+ // ==================== Binary expression codegen ====================
+
+ /**
+ * JLS-style numeric type tag used for binary numeric promotion. Ordered
+ * narrow → wide, so {@code commonType(a, b)} can return the wider of two.
+ */
+ private enum ExprType {
+ INT, LONG, FLOAT, DOUBLE,
+ STRING, BOOLEAN, OBJECT, UNKNOWN;
+
+ boolean isNumeric() {
+ return this == INT || this == LONG || this == FLOAT || this ==
DOUBLE;
+ }
+ }
+
+ /**
+ * Parsed representation of a NUMBER literal token. Carries the inferred
+ * Java type and the literal text trimmed/suffixed for direct emission as
+ * Java source.
+ */
+ private static final class NumericLiteral {
+ final ExprType type;
+ /** Java-source representation, e.g. "10000", "10000L", "1.5", "1.5f".
*/
+ final String javaText;
+
+ private NumericLiteral(final ExprType type, final String javaText) {
+ this.type = type;
+ this.javaText = javaText;
+ }
+
+ static NumericLiteral parse(final String numText) {
+ String t = numText;
+ char suffix = 0;
+ if (!t.isEmpty()) {
+ final char last = t.charAt(t.length() - 1);
+ if (last == 'L' || last == 'l' || last == 'F' || last == 'f'
+ || last == 'D' || last == 'd') {
+ suffix = last;
+ t = t.substring(0, t.length() - 1);
+ }
+ }
+ final boolean fractional = t.contains(".") || t.contains("e") ||
t.contains("E");
+ switch (suffix) {
+ case 'L':
+ case 'l':
+ return new NumericLiteral(ExprType.LONG, t + "L");
+ case 'F':
+ case 'f':
+ return new NumericLiteral(ExprType.FLOAT,
+ (fractional ? t : (t + ".0")) + "f");
+ case 'D':
+ case 'd':
+ return new NumericLiteral(ExprType.DOUBLE,
+ (fractional ? t : (t + ".0")) + "d");
+ default:
+ if (fractional) {
+ return new NumericLiteral(ExprType.DOUBLE, t);
+ }
+ // Bare integer literal — INT if it fits, else LONG.
+ try {
+ Integer.parseInt(t);
+ return new NumericLiteral(ExprType.INT, t);
+ } catch (NumberFormatException e) {
+ return new NumericLiteral(ExprType.LONG, t + "L");
+ }
+ }
+ }
+ }
+
/**
- * Returns {@code true} when every concat part is numeric — i.e. a
- * parenthesized expression with an {@code Integer} or {@code Long} cast,
- * or a bare number literal. If so, {@code +} is arithmetic, not string
- * concatenation.
+ * Returns the JLS-style binary numeric promotion result for the two
operand
+ * types. Both must be numeric; otherwise this is a programmer bug (the
+ * caller is responsible for the non-numeric error path).
*/
- private static boolean allPartsNumeric(final
List<LALScriptModel.ValueAccess> parts) {
- for (final LALScriptModel.ValueAccess part : parts) {
- if (part.isNumberLiteral()) {
+ private static ExprType promote(final ExprType a, final ExprType b) {
+ if (a == ExprType.DOUBLE || b == ExprType.DOUBLE) {
+ return ExprType.DOUBLE;
+ }
+ if (a == ExprType.FLOAT || b == ExprType.FLOAT) {
+ return ExprType.FLOAT;
+ }
+ if (a == ExprType.LONG || b == ExprType.LONG) {
+ return ExprType.LONG;
+ }
+ return ExprType.INT;
+ }
+
+ private static String javaPrimitiveName(final ExprType t) {
+ switch (t) {
+ case LONG:
+ return "long";
+ case FLOAT:
+ return "float";
+ case DOUBLE:
+ return "double";
+ case INT:
+ default:
+ return "int";
+ }
+ }
+
+ private static String javaWrapperName(final ExprType t) {
+ switch (t) {
+ case LONG:
+ return "Long";
+ case FLOAT:
+ return "Float";
+ case DOUBLE:
+ return "Double";
+ case INT:
+ default:
+ return "Integer";
+ }
+ }
+
+ private static Class<?> primitiveClass(final ExprType t) {
+ switch (t) {
+ case LONG:
+ return long.class;
+ case FLOAT:
+ return float.class;
+ case DOUBLE:
+ return double.class;
+ case INT:
+ default:
+ return int.class;
+ }
+ }
+
+ /**
+ * Compile-time inference of an operand's Java type. Operates on the same
+ * AST that codegen will subsequently emit, so the result is consistent.
+ */
+ private static ExprType inferType(final LALScriptModel.ValueAccess part,
+ final LALClassGenerator.GenCtx genCtx) {
+ if (part == null) {
+ return ExprType.UNKNOWN;
+ }
+ if (part.isStringLiteral()) {
+ return ExprType.STRING;
+ }
+ if (part.isNumberLiteral() && part.getChain().isEmpty()) {
+ return NumericLiteral.parse(part.getSegments().get(0)).type;
+ }
+ // (expr as Cast) primary — only trustable when no further chain is
applied.
+ if (part.getParenInner() != null && part.getChain().isEmpty()) {
+ if (part.getParenCast() != null) {
+ return castToType(part.getParenCast());
+ }
+ // Pure grouping (no cast): the parens just preserve precedence.
+ return inferType(part.getParenInner(), genCtx);
+ }
+ // Nested binary expression: recompute via its parts/ops.
+ if (!part.getConcatParts().isEmpty()) {
+ return inferBinaryType(part.getConcatParts(), part.getConcatOps(),
genCtx);
+ }
+ // def variable with a known resolved type.
+ if (!part.getSegments().isEmpty()) {
+ final LALClassGenerator.LocalVarInfo lv =
+ genCtx.localVars.get(part.getSegments().get(0));
+ if (lv != null && part.getChain().isEmpty()) {
+ final Class<?> t = lv.resolvedType;
+ if (t == int.class || t == Integer.class) {
+ return ExprType.INT;
+ }
+ if (t == long.class || t == Long.class) {
+ return ExprType.LONG;
+ }
+ if (t == float.class || t == Float.class) {
+ return ExprType.FLOAT;
+ }
+ if (t == double.class || t == Double.class) {
+ return ExprType.DOUBLE;
+ }
+ if (t == String.class) {
+ return ExprType.STRING;
+ }
+ if (t == boolean.class || t == Boolean.class) {
+ return ExprType.BOOLEAN;
+ }
+ return ExprType.OBJECT;
+ }
+ }
+ // tag() / sourceAttribute() — always return String at runtime.
+ if ("tag".equals(part.getFunctionCallName())
+ || "sourceAttribute".equals(part.getFunctionCallName())) {
+ return ExprType.STRING;
+ }
+ // parsed.* — when the rule has a typed inputType (no JSON/YAML/TEXT
+ // parser), walk the proto getter chain via reflection so primitive
+ // numeric leaves participate in arithmetic and don't fall through
+ // to string concat. Mirrors {@link #generateExtraLogAccess} but
+ // does not emit code or local variables.
+ if (part.isParsedRef()
+ && genCtx.parserType == LALClassGenerator.ParserType.NONE
+ && genCtx.inputType != null) {
+ final Class<?> primitive = walkProtoChainPrimitiveType(
+ part.getChain(), genCtx.inputType);
+ if (primitive != null) {
+ if (primitive == int.class) {
+ return ExprType.INT;
+ }
+ if (primitive == long.class) {
+ return ExprType.LONG;
+ }
+ if (primitive == float.class) {
+ return ExprType.FLOAT;
+ }
+ if (primitive == double.class) {
+ return ExprType.DOUBLE;
+ }
+ if (primitive == boolean.class) {
+ return ExprType.BOOLEAN;
+ }
+ }
+ }
+ return ExprType.UNKNOWN;
+ }
+
+ /**
+ * Walk a {@code parsed.*} field-segment chain over the typed proto root
+ * via reflection, returning the primitive return type of the final getter
+ * if the chain is field-only and ends at a primitive accessor; {@code
null}
+ * otherwise.
+ */
+ private static Class<?> walkProtoChainPrimitiveType(
+ final List<LALScriptModel.ValueAccessSegment> chain,
+ final Class<?> rootType) {
+ if (chain == null || chain.isEmpty()) {
+ return null;
+ }
+ Class<?> currentType = rootType;
+ for (int i = 0; i < chain.size(); i++) {
+ final LALScriptModel.ValueAccessSegment seg = chain.get(i);
+ if (!(seg instanceof LALScriptModel.FieldSegment)) {
+ return null;
+ }
+ final String field = ((LALScriptModel.FieldSegment) seg).getName();
+ final String getter = "get" +
Character.toUpperCase(field.charAt(0))
+ + field.substring(1);
+ try {
+ currentType = currentType.getMethod(getter).getReturnType();
+ } catch (NoSuchMethodException e) {
+ return null;
+ }
+ }
+ return currentType.isPrimitive() ? currentType : null;
+ }
+
+ private static ExprType castToType(final String cast) {
+ if ("Integer".equals(cast)) {
+ return ExprType.INT;
+ }
+ if ("Long".equals(cast)) {
+ return ExprType.LONG;
+ }
+ if ("Float".equals(cast)) {
+ return ExprType.FLOAT;
+ }
+ if ("Double".equals(cast)) {
+ return ExprType.DOUBLE;
+ }
+ if ("String".equals(cast)) {
+ return ExprType.STRING;
+ }
+ if ("Boolean".equals(cast)) {
+ return ExprType.BOOLEAN;
+ }
+ return ExprType.UNKNOWN;
+ }
+
+ /**
+ * Walk the operand list applying JLS promotion left-to-right, treating any
+ * {@code +} that touches a String operand as string concat (which sticks).
+ */
+ private static ExprType inferBinaryType(
+ final List<LALScriptModel.ValueAccess> parts,
+ final List<LALScriptModel.BinaryOp> ops,
+ final LALClassGenerator.GenCtx genCtx) {
+ ExprType acc = inferType(parts.get(0), genCtx);
+ for (int i = 1; i < parts.size(); i++) {
+ final ExprType rhs = inferType(parts.get(i), genCtx);
+ final LALScriptModel.BinaryOp op = ops.get(i - 1);
+ if (op == LALScriptModel.BinaryOp.PLUS
+ && (acc == ExprType.STRING || rhs == ExprType.STRING)) {
+ acc = ExprType.STRING;
+ continue;
+ }
+ if (acc.isNumeric() && rhs.isNumeric()) {
+ acc = promote(acc, rhs);
continue;
}
- if (part.getParenInner() != null &&
isNumericCast(part.getParenCast())) {
+ // Anything else for + (object + object) → fallback to string
concat
+ // (preserves long-standing behaviour for `tag("a") + tag("b")`
etc.).
+ if (op == LALScriptModel.BinaryOp.PLUS) {
+ acc = ExprType.STRING;
continue;
}
- return false;
+ // - * / on non-numeric — caller emits a compile error.
+ return ExprType.UNKNOWN;
}
- return true;
+ return acc;
}
- private static boolean isNumericCast(final String cast) {
- return "Integer".equals(cast) || "Long".equals(cast);
+ /**
+ * Top-level binary-expression codegen. Mirrors Java's left-to-right
+ * evaluation: adjacent numeric operands accumulate into a primitive
+ * arithmetic expression; once a String operand appears, every subsequent
+ * {@code +} becomes string concatenation. This preserves the semantic
+ * difference between {@code 1 + 2 + "x"} (= {@code "3x"}) and
+ * {@code "" + 1 + 2 + "x"} (= {@code "12x"}).
+ */
+ private static void generateBinaryExpression(
+ final StringBuilder sb,
+ final List<LALScriptModel.ValueAccess> parts,
+ final List<LALScriptModel.BinaryOp> ops,
+ final LALClassGenerator.GenCtx genCtx) {
+ // Render the first operand and seed the accumulator with its type.
+ ExprType accType = inferType(parts.get(0), genCtx);
+ final StringBuilder accBuf = new StringBuilder();
+ appendOperandRaw(accBuf, parts.get(0), accType, genCtx);
+ String accExpr = accBuf.toString();
+
+ for (int i = 1; i < parts.size(); i++) {
+ final LALScriptModel.BinaryOp op = ops.get(i - 1);
+ final ExprType rhsType = inferType(parts.get(i), genCtx);
+ final StringBuilder rhsBuf = new StringBuilder();
+ appendOperandRaw(rhsBuf, parts.get(i), rhsType, genCtx);
+ final String rhsExpr = rhsBuf.toString();
+
+ if (accType.isNumeric() && rhsType.isNumeric()) {
+ final ExprType promoted = promote(accType, rhsType);
+ final String widenedAcc = widenPrimitiveExpr(accExpr, accType,
promoted);
+ final String widenedRhs = widenPrimitiveExpr(rhsExpr, rhsType,
promoted);
+ accExpr = "(" + widenedAcc + " " + opSymbol(op) + " " +
widenedRhs + ")";
+ accType = promoted;
+ continue;
+ }
+ if (op != LALScriptModel.BinaryOp.PLUS) {
+ throw new IllegalArgumentException(
+ "Operator '" + opSymbol(op) + "' requires numeric
operands; "
+ + "got non-numeric expression. Cast operands with "
+ + "'as Integer/Long/Float/Double' to enable
arithmetic.");
+ }
+ // String concat path. Coerce the accumulator to String when
+ // needed: a STRING accumulator is fine; a numeric accumulator
+ // is already in parens (so Java evaluates it before the concat,
+ // preserving `(1 + 2) + "x"` → `"3x"`); anything else needs a
+ // leading "" to make Java accept Object + Object.
+ if (accType != ExprType.STRING && !accType.isNumeric()) {
+ accExpr = "\"\" + " + accExpr;
+ }
+ accExpr = accExpr + " + " + rhsExpr;
+ accType = ExprType.STRING;
+ }
+
+ if (accType.isNumeric()) {
+ // Expose the primitive form so the caller (numeric comparison)
+ // can avoid h.toX() unboxing; emit a boxed form into sb for
+ // Object contexts (tag assignments, def initialisers, ...).
+ genCtx.lastResolvedType = primitiveClass(accType);
+ genCtx.lastRawChain = accExpr;
+ genCtx.lastNullChecks = null;
+
sb.append(javaWrapperName(accType)).append(".valueOf(").append(accExpr).append(")");
+ } else {
Review Comment:
When a binary expression ends up as string concatenation, this method never
overwrites `genCtx.lastResolvedType` / `lastRawChain`. `generateDefStatement`
uses `lastResolvedType` to declare local variables, so a valid rule like `def
msg = "count=" + (tag("n") as Integer)` is inferred as `Integer` and fails
compilation. Please record a non-numeric result type here before returning the
string expression.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java:
##########
@@ -163,29 +165,240 @@ static void generateNumericComparison(
final LALScriptModel.ComparisonCondition cc,
final String op,
final LALClassGenerator.GenCtx genCtx) {
- // Generate left side into buffer to inspect resolved type
+ // Decide the primitive comparison type from explicit casts and the
+ // operand shapes — both sides influence the choice via JLS-style
+ // promotion. RHS-side cast is read off ValueAccessConditionValue;
+ // RHS literals contribute via their inferred numeric type.
+ final ExprType lhsType = inferComparisonOperandType(cc.getLeft(),
cc.getLeftCast(), genCtx);
+ final ExprType rhsType = inferRightHandType(cc.getRight(), genCtx);
+ final ExprType cmpType = pickComparisonType(lhsType, rhsType);
+ final Class<?> cmpClass = primitiveClass(cmpType);
+
+ // Render the LHS, then capture its primitive metadata BEFORE the RHS
+ // generator overwrites genCtx fields. A top-level numeric cast on
+ // the comparison (`tag("a") as Integer < 1.5`) is honoured first so
+ // the operand is rendered in the user's declared primitive, not in
+ // the promoted comparison type.
+ genCtx.lastNullChecks = null;
final StringBuilder leftBuf = new StringBuilder();
- generateValueAccessObj(leftBuf, cc.getLeft(), null, genCtx);
-
- final boolean primitiveNumeric = genCtx.lastResolvedType != null
- && (genCtx.lastResolvedType == int.class
- || genCtx.lastResolvedType == long.class);
-
- if (primitiveNumeric && genCtx.lastRawChain != null) {
- // Direct primitive comparison — no boxing, no h.toLong()
- if (genCtx.lastNullChecks != null) {
- sb.append("(").append(genCtx.lastNullChecks).append(" ? false
: ")
- .append(genCtx.lastRawChain).append(op);
- generateConditionValueNumeric(sb, cc.getRight(), genCtx);
- sb.append(")");
- } else {
- sb.append(genCtx.lastRawChain).append(op);
- generateConditionValueNumeric(sb, cc.getRight(), genCtx);
- }
+ emitOperandRespectingCast(leftBuf, cc.getLeft(), cc.getLeftCast(),
genCtx);
+ final Class<?> lhsResolved = genCtx.lastResolvedType;
+ final String lhsRawChain = genCtx.lastRawChain;
+ final String lhsNullChecks = genCtx.lastNullChecks;
+ final boolean lhsPrimitive = lhsResolved != null
+ && (lhsResolved == int.class || lhsResolved == long.class
+ || lhsResolved == float.class || lhsResolved == double.class);
+
+ final String leftExpr;
+ if (lhsPrimitive && lhsRawChain != null) {
+ // Already primitive — widen to cmpType only if narrower.
+ leftExpr = widenPrimitiveExpr(lhsRawChain,
+ primitiveToExprType(lhsResolved), cmpType);
+ } else {
+ // Untyped operand — wrap with the helper that produces the
+ // chosen primitive type. h.toLong is no longer the universal
+ // fallback when the user wrote `as Double` / `as Float`.
+ leftExpr = wrapAsPrimitive(leftBuf.toString(), cmpType);
+ }
+
+ // Render the RHS into its own buffer so we can read back any null
+ // guards it produced (e.g. from a typed-proto chain like
+ // `parsed?.response?.responseCode?.value`) and apply them at the
+ // outer ternary alongside the LHS guards.
+ genCtx.lastNullChecks = null;
+ final StringBuilder rhsBuf = new StringBuilder();
+ generateConditionValueNumeric(rhsBuf, cc.getRight(), cmpClass, genCtx);
+ final String rhsNullChecks = genCtx.lastNullChecks;
+
+ final String combinedNullChecks = combineNullChecks(lhsNullChecks,
rhsNullChecks);
+ if (combinedNullChecks != null) {
+ sb.append("(").append(combinedNullChecks).append(" ? false : ")
+ .append(leftExpr).append(op).append(rhsBuf).append(")");
} else {
- // Fallback: h.toLong() conversion
- sb.append("h.toLong(").append(leftBuf).append(")").append(op);
- generateConditionValueNumeric(sb, cc.getRight(), genCtx);
+ sb.append(leftExpr).append(op).append(rhsBuf);
+ }
+ }
+
+ /**
+ * Combine LHS / RHS null-guard expressions for a comparison. Either side
+ * may be {@code null} (no guard); the result is the disjunction so the
+ * comparison short-circuits to {@code false} as soon as any participant
+ * is null, mirroring the previous single-side behaviour.
+ */
+ private static String combineNullChecks(final String lhs, final String
rhs) {
+ if (lhs == null && rhs == null) {
+ return null;
+ }
+ if (lhs == null) {
+ return rhs;
+ }
+ if (rhs == null) {
+ return lhs;
+ }
+ return lhs + " || " + rhs;
+ }
+
+ /**
+ * Read the numeric type the user declared for the left side of a
+ * comparison: top-level {@code as} cast on the comparison wins, otherwise
+ * fall back to inspecting the operand AST.
+ */
+ private static ExprType inferComparisonOperandType(
+ final LALScriptModel.ValueAccess value,
+ final String topLevelCast,
+ final LALClassGenerator.GenCtx genCtx) {
+ final ExprType fromCast = castToType(topLevelCast);
+ if (fromCast.isNumeric()) {
+ return fromCast;
+ }
+ final ExprType inferred = inferType(value, genCtx);
+ return inferred.isNumeric() ? inferred : ExprType.LONG;
+ }
+
+ /**
+ * Read the numeric type of a comparison's RHS — literal type for number
+ * literals, declared cast for value accesses, otherwise unknown.
+ */
+ private static ExprType inferRightHandType(
+ final LALScriptModel.ConditionValue cv,
+ final LALClassGenerator.GenCtx genCtx) {
+ if (cv instanceof LALScriptModel.NumberConditionValue) {
+ final String literal = ((LALScriptModel.NumberConditionValue)
cv).getLiteral();
+ return literal != null ? NumericLiteral.parse(literal).type :
ExprType.LONG;
+ }
+ if (cv instanceof LALScriptModel.ValueAccessConditionValue) {
+ final LALScriptModel.ValueAccessConditionValue vacv =
+ (LALScriptModel.ValueAccessConditionValue) cv;
+ final ExprType castT = castToType(vacv.getCastType());
+ if (castT.isNumeric()) {
+ return castT;
+ }
+ return inferType(vacv.getValue(), genCtx);
+ }
Review Comment:
This new RHS type inference is never reached for `==` / `!=` when the
right-hand side is another value access. `generateCondition()` still routes
those cases through `Objects.equals`, so rules like `tag("a") as Integer ==
tag("b") as Integer` ignore both numeric casts and compare the original
string/object forms instead of numeric values.
--
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]
