Copilot commented on code in PR #13858:
URL: https://github.com/apache/skywalking/pull/13858#discussion_r3178288229
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java:
##########
@@ -163,29 +165,282 @@ 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 {
- // Fallback: h.toLong() conversion
- sb.append("h.toLong(").append(leftBuf).append(")").append(op);
- generateConditionValueNumeric(sb, cc.getRight(), genCtx);
+ // 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 {
+ 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;
+ }
+
+ /**
+ * Decide whether an {@code ==} / {@code !=} comparison should be
+ * routed through numeric primitive comparison rather than
+ * {@code Objects.equals}. True when BOTH sides resolve to a numeric
+ * type (via explicit cast, paren-cast, or AST inference). A numeric
+ * literal RHS is enough on its own — Java promotes the LHS to match.
+ */
+ private static boolean isNumericComparison(
+ final LALScriptModel.ComparisonCondition cc,
+ final LALClassGenerator.GenCtx genCtx) {
+ if (cc.getRight() instanceof LALScriptModel.NumberConditionValue) {
+ return true;
+ }
+ if (!(cc.getRight() instanceof
LALScriptModel.ValueAccessConditionValue)) {
+ return false;
+ }
+ final LALScriptModel.ValueAccessConditionValue vacv =
+ (LALScriptModel.ValueAccessConditionValue) cc.getRight();
+ return numericTypeOrNull(cc.getLeft(), cc.getLeftCast(), genCtx) !=
null
+ && numericTypeOrNull(vacv.getValue(), vacv.getCastType(), genCtx)
!= null;
+ }
+
+ /**
+ * Return the operand's numeric {@link ExprType} if and only if it
+ * resolves to one of {@code INT / LONG / FLOAT / DOUBLE} via an
+ * explicit cast or compile-time inference; otherwise {@code null}.
+ * Used by {@link #isNumericComparison} to decide whether {@code ==} /
+ * {@code !=} should compare primitives — note this is strict (no
+ * {@code LONG} fallback like {@link #inferComparisonOperandType}).
+ */
+ private static ExprType numericTypeOrNull(
+ 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 : null;
+ }
+
+ /**
+ * 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);
+ }
+ return ExprType.UNKNOWN;
+ }
+
+ /**
+ * Pick the JLS-promoted primitive type for a comparison given both sides'
+ * inferred types. Numeric sides drive promotion; non-numeric sides default
+ * to {@code LONG} so the comparison still has a primitive home.
+ */
+ private static ExprType pickComparisonType(final ExprType lhs, final
ExprType rhs) {
+ if (lhs.isNumeric() && rhs.isNumeric()) {
+ return promote(lhs, rhs);
+ }
+ if (lhs.isNumeric()) {
+ return lhs;
+ }
+ if (rhs.isNumeric()) {
+ return rhs;
+ }
+ return ExprType.LONG;
+ }
+
+ /**
+ * Emit a primitive widening cast on an already-primitive expression, only
+ * when the source type is narrower than the target.
+ */
+ private static String widenPrimitiveExpr(final String expr,
+ final ExprType from,
+ final ExprType to) {
+ if (from == to || promote(from, to) == from) {
+ return expr;
+ }
+ return "(" + javaPrimitiveName(to) + ") " + expr;
+ }
+
+ /**
+ * Render an operand for a numeric comparison while preserving the user's
+ * declared cast. Two-phase to avoid double-coercion:
+ * <ol>
+ * <li>Render the operand expression. Typed proto fields, arithmetic
+ * results, and paren-cast operands already land as a primitive and
+ * record the type via {@code genCtx.lastResolvedType}.</li>
+ * <li>If the rendered expression is non-primitive (a boxed/Object)
+ * and the user declared a numeric cast, wrap it with the matching
+ * {@code h.toX()} helper so the primitive identity is preserved
+ * — e.g. {@code tag("a") as Integer < 1.5} renders
+ * {@code (double) h.toInt(h.tagValue("a")) < 1.5}, not
+ * {@code h.toDouble(h.tagValue("a")) < 1.5}.</li>
+ * </ol>
+ */
+ private static void emitOperandRespectingCast(
+ final StringBuilder sb,
+ final LALScriptModel.ValueAccess value,
+ final String castType,
+ final LALClassGenerator.GenCtx genCtx) {
+ final int start = sb.length();
+ generateValueAccessObj(sb, value, castType, genCtx);
+ final Class<?> resolved = genCtx.lastResolvedType;
+ final boolean alreadyPrimitive = resolved != null &&
resolved.isPrimitive();
+ final ExprType castET = castToType(castType);
+ if (alreadyPrimitive) {
+ // Native primitive. If the user declared a different numeric
+ // type, honour it via a Java primitive cast — e.g.
+ // `((tag("a") as Long) + 1) as Integer` should narrow the long
+ // sum to int rather than silently keep the long value. Read
+ // the primitive form from lastRawChain (sb may hold a boxed
+ // wrapper like `Long.valueOf(...)` that the verifier can't
+ // unbox via a primitive cast).
+ if (castET.isNumeric()
+ && primitiveToExprType(resolved) != castET
+ && genCtx.lastRawChain != null) {
+ final String primExpr = genCtx.lastRawChain;
+ sb.setLength(start);
+ sb.append("(").append(javaPrimitiveName(castET))
+ .append(") (").append(primExpr).append(")");
+ recordPrimitiveResult(genCtx, primitiveClass(castET), sb,
start);
+ }
Review Comment:
When a safe-nav typed-proto leaf is already primitive and then widened to a
different numeric cast here, `recordPrimitiveResult()` clears the null guard
that `generateExtraLogAccess()` recorded. A comparison like
`parsed?.response?.responseCode?.value as Long < 1` will therefore emit the
widened primitive access without the outer `== null ? false :` check and can
throw an NPE when the chain is missing.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java:
##########
@@ -997,6 +1813,14 @@ static String generateMethodArgs(
&& genCtx.localVars.containsKey(text)) {
// Local def variable reference
sb.append(genCtx.localVars.get(text).javaVarName);
+ } else if (genCtx != null
+ && (va.isParsedRef() || va.isLogRef()
+ || !va.getChain().isEmpty()
+ || va.getFunctionCallName() != null
+ || va.getParenInner() != null)) {
+ // tag(), parsed.*, log.*, paren-cast, or any value
+ // access with a chain — render via the standard path.
+ generateValueAccess(sb, va, genCtx);
Review Comment:
This branch renders method-call arguments with `generateValueAccess(...)`
but never applies `arg.getCastType()`. As a result, arguments like
`substring(tag("n") as Integer)` or `foo(parsed.value as Double)` are emitted
as the original String/Object expression instead of `h.toInt(...)` /
`h.toDouble(...)`, so overload resolution and runtime semantics are still wrong
for casted method arguments.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java:
##########
@@ -865,80 +1260,478 @@ 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;
+ }
+ }
+
/**
- * 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.
+ * 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 boolean allPartsNumeric(final
List<LALScriptModel.ValueAccess> parts) {
- for (final LALScriptModel.ValueAccess part : parts) {
- if (part.isNumberLiteral()) {
+ 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':
+ // Validate the body fits in a Java long — otherwise a
+ // huge `<digits>L` token would slip through and surface
+ // later as an opaque Javassist error.
+ try {
+ Long.parseLong(t);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(
+ "Long literal '" + numText + "' exceeds the "
+ + "supported range (must fit in a Java long)");
+ }
+ 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");
Review Comment:
The new `Float`/`Double` literal paths normalize the token text but never
validate that the magnitude is representable as a Java float/double. Literals
like `1e400` or `1e400f` will therefore pass parsing here and fail later when
Javassist compiles the generated Java, which is the same opaque failure mode
this method already avoids for oversized integer/long literals.
##########
oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/v2/compiler/LALValueCodegen.java:
##########
@@ -865,80 +1260,478 @@ 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;
+ }
+ }
+
/**
- * 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.
+ * 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 boolean allPartsNumeric(final
List<LALScriptModel.ValueAccess> parts) {
- for (final LALScriptModel.ValueAccess part : parts) {
- if (part.isNumberLiteral()) {
+ 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':
+ // Validate the body fits in a Java long — otherwise a
+ // huge `<digits>L` token would slip through and surface
+ // later as an opaque Javassist error.
+ try {
+ Long.parseLong(t);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(
+ "Long literal '" + numText + "' exceeds the "
+ + "supported range (must fit in a Java long)");
+ }
+ 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.
+ // Reject values that don't fit in Java's long range
+ // upfront so the parser raises a clear error rather
+ // than letting Javassist fail later on `<huge>L`.
+ try {
+ Integer.parseInt(t);
+ return new NumericLiteral(ExprType.INT, t);
+ } catch (NumberFormatException ignoredInt) {
+ try {
+ Long.parseLong(t);
+ return new NumericLiteral(ExprType.LONG, t + "L");
+ } catch (NumberFormatException ignoredLong) {
+ throw new IllegalArgumentException(
+ "Numeric literal '" + numText
+ + "' exceeds the supported range "
+ + "(must fit in a Java long; use a "
+ + "fractional or 'D'/'F'-suffixed form "
+ + "for larger magnitudes)");
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * 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 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 (part.getParenInner() != null &&
isNumericCast(part.getParenCast())) {
+ if (acc.isNumeric() && rhs.isNumeric()) {
+ acc = promote(acc, rhs);
continue;
}
- return false;
+ // 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;
+ }
+ // - * / 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. Java's `+` only accepts the chain when
+ // at least one operand is statically a String; otherwise (e.g.
+ // `int + Object` or `Object + Object`) the source won't even
+ // compile. Prepend a leading `""` whenever neither side is
+ // statically String at the transition point. The numeric
+ // arithmetic prefix is already in parens, so `("" + (1 + 2))`
+ // still computes the sum first ("3"), preserving the
+ // semantics of `1 + 2 + parsed.x` (= "3<obj>").
+ if (accType != ExprType.STRING && rhsType != ExprType.STRING) {
+ 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 {
+ // Non-numeric end state: a String concatenation. Update the
+ // genCtx metadata explicitly — otherwise a stale numeric
+ // resolved type from an inner operand would leak out and
+ // mis-type things like `def msg = "count=" + (tag("n") as
Integer)`,
+ // which the def codegen reads from lastResolvedType to declare
+ // the local variable.
+ genCtx.lastResolvedType = String.class;
+ genCtx.lastRawChain = null;
+ genCtx.lastNullChecks = null;
+ sb.append(accExpr);
+ }
}
/**
- * Generates an arithmetic sum expression for a list of numeric parts.
- * Always uses {@code long} arithmetic to avoid Javassist autoboxing
- * restrictions (Javassist cannot pass a primitive {@code int/long} to a
- * method that expects {@code Object}, e.g. {@code h.toLong(int)}).
- *
- * <p>Two outputs are produced:
- * <ul>
- * <li>The raw {@code long} expression is stored in
- * {@code genCtx.lastRawChain} so that {@code
generateNumericComparison}
- * can emit a direct primitive comparison without a
- * {@code h.toLong()} wrapper.</li>
- * <li>{@code Long.valueOf(rawExpr)} is appended to {@code sb} so that
- * non-comparison contexts (tag assignment, {@code h.toStr()}, etc.)
- * receive a boxed {@code Long} — a valid {@code Object}.</li>
- * </ul>
- *
- * <p>Examples:
- * <pre>{@code
- * (tag("a") as Integer) + (tag("b") as Integer) < 10000
- * rawExpr → ((long) h.toInt(h.tagValue("a")) + (long)
h.toInt(h.tagValue("b")))
- * in sb → Long.valueOf(((long) h.toInt(...) + (long) h.toInt(...)))
- * comparison emits → rawExpr < 10000L (via lastRawChain /
primitiveNumeric path)
- * }</pre>
+ * Render a single operand for use inside a binary expression — returning
+ * its raw form (primitive expression for numeric operands, Object/String
+ * form otherwise). Number literals emit with their declared suffix,
+ * paren-cast operands emit through the matching helper, paren-grouping
+ * recurses into the inner expression, and nested binary expressions
+ * reuse the inner accumulator's raw chain when numeric.
*/
- private static void generateArithmeticSum(final StringBuilder sb,
- final
List<LALScriptModel.ValueAccess> parts,
- final LALClassGenerator.GenCtx
genCtx) {
- final StringBuilder expr = new StringBuilder("(");
- for (int i = 0; i < parts.size(); i++) {
- if (i > 0) {
- expr.append(" + ");
+ private static void appendOperandRaw(final StringBuilder sb,
+ final LALScriptModel.ValueAccess
part,
+ final ExprType partType,
+ final LALClassGenerator.GenCtx
genCtx) {
+ if (part.isNumberLiteral() && part.getChain().isEmpty()) {
+
sb.append(NumericLiteral.parse(part.getSegments().get(0)).javaText);
+ return;
+ }
+ if (part.getParenInner() != null && part.getParenCast() != null
+ && part.getChain().isEmpty() && partType.isNumeric()) {
+ generateCastedValueAccess(sb, part.getParenInner(),
+ javaWrapperName(partType), genCtx);
+ return;
+ }
+ if (part.getParenInner() != null && part.getParenCast() == null
+ && part.getChain().isEmpty()) {
+ final StringBuilder inner = new StringBuilder();
+ generateValueAccessObj(inner, part.getParenInner(), null, genCtx);
+ final Class<?> resolved = genCtx.lastResolvedType;
+ if (partType.isNumeric() && resolved != null &&
resolved.isPrimitive()
+ && genCtx.lastRawChain != null) {
+ sb.append(genCtx.lastRawChain);
+ } else {
+ sb.append(inner);
}
- final LALScriptModel.ValueAccess part = parts.get(i);
- if (part.isNumberLiteral()) {
- expr.append(part.getSegments().get(0)).append("L");
- } else if ("Long".equals(part.getParenCast())) {
- generateCastedValueAccess(expr, part.getParenInner(), "Long",
genCtx);
+ return;
+ }
+ if (!part.getConcatParts().isEmpty()) {
+ final StringBuilder inner = new StringBuilder();
+ generateBinaryExpression(inner, part.getConcatParts(),
+ part.getConcatOps(), genCtx);
+ if (partType.isNumeric() && genCtx.lastRawChain != null) {
+ sb.append(genCtx.lastRawChain);
} else {
- expr.append("(long) ");
- generateCastedValueAccess(expr, part.getParenInner(),
"Integer", genCtx);
+ sb.append(inner);
}
+ return;
+ }
+ if (partType.isNumeric()) {
+ // Resolve untyped operand (e.g. tag(), parsed.x) into the
+ // expected primitive via h.toX().
+ generateCastedValueAccess(sb, part, javaWrapperName(partType),
genCtx);
Review Comment:
For numeric operands we always force the value through
`generateCastedValueAccess(...)`. On a safe-nav typed-proto primitive leaf, the
inner access is `(... == null ? null : Integer.valueOf(raw))`, and
`h.toInt(null)` / `h.toLong(null)` then turns that missing value into `0`. So
expressions like `parsed?.response?.responseCode?.value + 1` silently evaluate
as `1` when the chain is absent instead of propagating the null-safe miss.
--
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]
