Generated code for refined methods with generic type declarations is missing return type

Issue #277 resolved
Zimon Kuhs
created an issue

When refining a method with generic types, the generated code is wrong.

With this aspect

aspect Test {
    public class Klass {}
    private static <T extends List<?>> T Klass.method() { return new ArrayList<String>(); }
    refine Test private static T Klass.method() { return refined(); }
}

the following code is generated (note that the return type T is appended to the method name of the method being refined instead of being a return type):

public class Klass extends java.lang.Object {
    private static <T extends List<?>> refined_Test_Klass_T method() { return new ArrayList<String>(); }
    private static T method() { return refined_Test_Klass_T(); }
}

which of course leads to the following errors:

error: cannot find symbol
    private static <T extends List<?>> refined_Test_Klass_T method() { return new ArrayList<String>(); }
                                       ^
  symbol:   class refined_Test_Klass_T
  location: class Klass
error: cannot find symbol
      private static T method() { return refined_Test_Klass_T(); }
                     ^
  symbol:   class T
  location: class Klass
error: cannot find symbol
      private static T method() { return refined_Test_Klass_T(); }
                                         ^
  symbol:   method refined_Test_Klass_T()
  location: class Klass

As a side note, JastAdd 2's reference documentation does not specify whether or not the type declaration is required in the refine-statement or not.

Comments (5)

  1. Jesper Öqvist

    Thank you for reporting this issue, and thank you for providing the test case! I can confirm that the test case fails in JastAdd2 2.2.2-39-g25b8f35.

    I added the test to the regression test suite as refine/generic_method_01p.

    The refine feature in JastAdd is one of the least polished features, in my opinion. The code that performs refinement is very complicated, and it does some really hacky stuff:

    • it accesses values in the parse tree by index in many places, and,
    • directly replaces literals in the parse tree to perform a refinement.

    This refinement process is very fragile to small changes in the parse tree. In the current issue, the name of the method to be refined is taken by indexing a specific node in the parse tree. It seems like the type parameter part of the original declaration (<T extends List<?>>) has shifted the node containing the method name so that instead the return type T is taken as the method name.

    Here is the relevant part of src/jastadd/ast/Attributes.jrag, in TypeDecl.refineWith(ClassBodyObject,ClassBodyObject):

        } else if (node instanceof org.jastadd.jrag.AST.ASTAspectMethodDeclaration
            || node instanceof org.jastadd.jrag.AST.ASTAspectRefineMethodDeclaration) {
          // Retrieve methodName.
          String idDecl = ((org.jastadd.jrag.AST.SimpleNode)decl.node.jjtGetChild(1)).firstToken.image;
          String methodName = idDecl.trim();
    
          // Add prefix refined_aspectName_.
          idDecl = idDecl.replaceAll(methodName,
              "refined_" + decl.aspectName() + "_" + name() + "_" + methodName);
          ((org.jastadd.jrag.AST.SimpleNode)decl.node.jjtGetChild(1)).firstToken.image = idDecl;
    
  2. Jesper Öqvist

    Fix error in generic method refinement

    Fixed an error in how refined method names are computed and modified by the method refinement process. See JastAdd issue #277.

    The parsing production for aspect method refinements has been updated to include optional type parameters.

    Added note in the reference manual about modifiers/type parameters/return type for method refinements.

    fixes #277 (bitbucket)

    → <<cset 4fed86ac9c59>>

  3. Jesper Öqvist

    @Zimon Kuhs I had to change some things in your test case to make it compile after the fix:

    // Refining works for generic methods.
    // https://bitbucket.org/jastadd/jastadd2/issues/277/generated-code-for-refined-methods-with
    // .grammar: { R; }
    // .result: COMPILE_PASS
    import java.util.*;
    
    aspect Test {
      public class Klass {}
    
      public static <T extends List<?>> T Klass.method() {
        return null;
      }
    
      refine Test public static <T extends List<?>> T Klass.method() {
        return refined();
      }
    
      public void R.foo() {
        // Simple use of the method to test that it was actually generated.
        Klass.method();
      }
    }
    

    Changes:

    • Added type parameters in the refine statement.
    • Removed new ArrayList<String>().
    • Added import statement.
  4. Zimon Kuhs reporter

    @Jesper Öqvist it makes sense to put the type declaration explicitly in the refine statement anyway. Well done, and thanks a lot for updating the reference manual. It will help new guys like me down the line!

    I hate to be a nitpicker but I noticed a spelling error in your commit's documentation update (those buggers can live forever), line 833 in doc/reference-manual.md, "paramgers": https://bitbucket.org/jastadd/jastadd2/commits/4fed86ac9c59f107e1278b58e09f9350a28aece0

  5. Log in to comment