Inconsistent behavior for new array of Number with numeric initializers

[BladeWise]

Numeric types are not handled consistently when trying to initialize arrays.

In general, trying to initialize a Number[] fails with both primitive and reference numeric types, but with different errors.

Below a couple of test to reproduce the issue:

    @Test
    public void newNumberArrayWithPrimitiveNumeric() {
        final Number[] array = new Number[] { 3, (Double) 0.9, (Number) 1.12f };

        final Expression newArray =
            Expression.newArrayInit(
                Types.Number,
                Expression.constant(3, PrimitiveTypes.Integer),
                Expression.constant(0.9, Types.Double),
                Expression.constant(1.12f, Types.Number)
            );
        final LambdaExpression<Supplier<Number[]>> lambda = Expression.lambda(Type.of(Supplier.class).makeGenericType(Types.Number.makeArrayType()), newArray);
        final Supplier<Number[]> delegate = lambda.compile();
        assertEquals(array[2], delegate.get()[2]);
    }

    @Test
    public void newNumberArrayWithoutPrimitiveNumeric() {
        final Number[] array = new Number[] { (Integer) 3, (Double) 0.9, (Number) 1.12f };

        final Expression newArray =
            Expression.newArrayInit(
                Types.Number,
                Expression.constant((Integer)3, Types.Integer),
                Expression.constant(0.9, Types.Double),
                Expression.constant(1.12f, Types.Number)
            );
        final LambdaExpression<Supplier<Number[]>> lambda = Expression.lambda(Type.of(Supplier.class).makeGenericType(Types.Number.makeArrayType()), newArray);
        final Supplier<Number[]> delegate = lambda.compile();
        assertEquals(array[2], delegate.get()[2]);
    }

Attached the full test class.

Notice that both initializations (with or without primitive types) are legal in Java, but in Procyon

• for primitive types, the Expression throws an exception (i.e. "An expression of type 'int' cannot be used to initialize an array of type 'java.lang.Number'", even if it is a superclass of the boxed value)
• for reference types, the Expression accepts the values, but an assertion in the CodeGenerator fails (actually, complaining about emitting a constant for the values provided as initializer).

The tests show that using an Object[] results in the same behavior, while assignment between Number/Object and numeric primitives/references works fine.

[BladeWise]

Upon further investigation, I was able to make the code work applying the following changes to the code base:

• Comment out this assert https://github.com/mstrobel/procyon/blob/fec0d7ae031403b4a56fc2f2fd801fc6d416305a/Procyon.Expressions/src/main/java/com/strobel/expressions/BoundConstants.java#L65-L67 which does not take into account a conversion from reference numeric (e.g. Double) to the Number superclass, in other words the calling code https://github.com/mstrobel/procyon/blob/fec0d7ae031403b4a56fc2f2fd801fc6d416305a/Procyon.Expressions/src/main/java/com/strobel/expressions/LambdaCompiler.java#L1997-L2026 performs a check which is not equivalent to the assertion (i.e. for Double to Number conversion, both isBoxed and isClosureAvailable are true, so generator.emitConstant is not called.

• Modify the conversion check in https://github.com/mstrobel/procyon/blob/fec0d7ae031403b4a56fc2f2fd801fc6d416305a/Procyon.Reflection/src/main/java/com/strobel/reflection/emit/CodeGenerator.java#L1799-L1805 in

      if (sourceType.isInterface() ||   // interface cast
          targetType.isInterface() ||
          sourceType == Types.Object ||   // boxing cast
          targetType == Types.Object ||
          (!targetType.isPrimitive() && !sourceType.isPrimitive() && TypeUtils.isNumeric(sourceType) && targetType.isAssignableFrom(sourceType))) { // super cast (e.g. Double -> Number)
  
          emitCastToType(sourceType, targetType);
      }

  to ensure that a cast is emitted for a numeric type to a superclass of the boxed reference.

• Relax the validation in newArrayInit https://github.com/mstrobel/procyon/blob/fec0d7ae031403b4a56fc2f2fd801fc6d416305a/Procyon.Expressions/src/main/java/com/strobel/expressions/Expression.java#L434-L436 to allow primitives with a matching boxed type

          if (!TypeUtils.areReferenceAssignable(elementType, TypeUtils.getBoxedTypeOrSelf(item.getType()))) {
              throw Error.expressionTypeCannotInitializeArrayType(item.getType(), elementType);
          }

With these modifications, the above tests complete (and no regression appears in existing ones, as far as I could see).

[mstrobel]

Will look into this in greater detail later, but this did catch my eye:

I don't think constant(1.12f, Types.Number) should be considered valid, and it's not really what the equivalent Java code does; it really ought to be convert(constant(1.12f), Types.Number), where the conversion is either implicit or explicit.

By the way, you've been submitting a lot of bug reports related to procyon-expressions (which is great!). May I ask how you've been using it? I get very little info about who's using the various Procyon libraries apart from the number of downloads on my decompiler.

[BladeWise]

To be honest, I think that an implicit conversion between primitive and Number is a bit of a stretch too, but it was quite handy! On the other side, reference types to Number seem legit.

In my case, I am using Procyon to implement some sort of configurable processing framework. Basically, I use a Java/C#-like scripting language to declare a processing pipeline, parse the configuration in an expression tree, and then let Procyon generate code at runtime.

I could have used a full-fledged scripting engine (or maybe Groovy), but this way I can implement some handy features to simplify/shorten the configuration or use language features not available in Java (my main background is C#, so conditional access operator, coalesce operator and extension methods are something I miss dearly).

[mstrobel]

Cool project! You may want to go ahead and check out my v0.6-staging branch, as it makes some changes to TypeBuilder and LambdaCompiler to let them work on JDKs newer than Java 8. Notably:

1. The name passed to a TypeBuilder is stripped of any package name, and only the class name is used.
2. The package is now provided via a MethodHandles.lookup(). If you don't provide one, it will search the base class and then the caller class for a lookup in a static field named PACKAGE_ACCESS or packageAccess. If it doesn't find one, it falls back to using a package called generated.
3. Lambdas basically follow the same rules, except to specify a lookup directly, you need to push an ExpressionContext.

These are, unfortunately, necessary, as the method I used to define classes dynamically has been deprecated and removed. As far as I can tell, you can no longer declare new classes in an arbitrary package (unless you resort to custom class loaders).

[BladeWise]

I had a look at that, and it seems really good! To be honest, I had already implemented a solution to port Procyon to JDK 11 using method handles (is a quick & dirty variation of what you do, and requires to provide an explicit lookup resolver), and was waiting for 0.6 release to migrate to your solution.

I have a fork here on GitHub, were I ported the project to Grade 7 (and was about to update JUnit to version 5), but I could not check signing. If you are interested, I can make a WIP PR to let you review and check it.