Multiple contribution to different targets

Issue #326 new
Idriss Riouak created an issue

If a contributor tries to contribute to two or more different targets, the result of each collection will be the union of all the contribution.

Let us consider the following example:

Root ::= A1:A A2:A B;
A;
B;

and

import java.util.*;
aspect Test {
    inh A B.a1();
    inh A B.a2();
    eq Root.getB().a1() = getA1();
    eq Root.getB().a2() = getA2();

    coll Collection<Integer> A.nbrs() [new ArrayList<Integer>()]
        with add root Root;
    B contributes 1 to A.nbrs() for a1();
    B contributes 2 to A.nbrs() for a2();
}

In this case B will contribute to a1().nbrs() with 1 and to a2().nbrs() with 2. But, after the evaluation of the attributes, we have a1().nbrs() == [1,2] == a2().nbrs().


The problem is due to how the contribution code is generated. Indeed, the generated code for B is the following one:

  protected void contributeTo_A_nbrs(Collection<Integer> collection) {
    super.contributeTo_A_nbrs(collection);
    collection.add(1);
    collection.add(2);
  }

This should be instead (can be improved):

  protected void contributeTo_A_nbrs(Collection<Integer> collection, A node) {
    super.contributeTo_A_nbrs(collection);
    if(node == a1()){
      collection.add(1);
    }else if(node == a2()){
      collection.add(2);
    }
  }

Attached you can find a test case that can be run with the command ./gradlew build

Comments (5)

  1. Jesper Öqvist

    It is not a great solution to evaluate a1() and a2() when adding the contributions, especially if this would happen for all collection attributes. In this particular case, the target attributes have already been evaluated when finding the contribution targets.

  2. Jesper Öqvist

    The workaround for this issue is to use @OnePhase

    @OnePhase coll Collection<Integer> A.nbrs() [new ArrayList<Integer>()];
    

  3. Niklas Fors

    It would be good with another test case that contains when conditions as well:

    B contributes 1 when cond1() to A.nbrs() for a1();
    B contributes 2 when cond2() to A.nbrs() for a1();
    B contributes 3 when cond3() to A.nbrs() for a2();
    B contributes 4 when cond4() to A.nbrs() for a2();
    
    syn boolean B.cond1() = true;
    syn boolean B.cond2() = false;
    syn boolean B.cond3() = true;
    syn boolean B.cond4() = false;
    

    By the way, the current implementation evaluates the when condition (cond1() etc) twice today, once during the survey phase and once during the contribution phase. The suggested solution would do this for the for expression as well. It might be possible with another solution that evaluates both the when and the for expression only once (maybe pass some unique identifier in each contribution statement or using closures).

  4. Jesper Öqvist

    I ran into this problem now. It was not obvious what was going wrong but then I remembered this issue.
    In ExtendJ there is an attribute nestedTypes() that collects classes nested inside a class. It looks like this:

      coll LinkedList<TypeDecl> TypeDecl.nestedTypes() root CompilationUnit;
    
      TypeDecl contributes this
          when isNestedType()
          to TypeDecl.nestedTypes()
          for enclosingType();
    

    I wanted to rewrite the handling of enum switch maps so that a nested class is generated. This required adding a contribution to nestedTypes() for the enum switch maps used inside a type so that they are generated in bytecode:

    TypeDecl contributes each enumSwitchMaps() to TypeDecl.nestedTypes() for this;
    

    Now the problem is that a TypeDecl always contributes enumSwitchMaps() to its own nested types, while the when isNestedType() contribution is only supposed to contribute to enclosingType() but there is no distinction between these two contributions in two-phase evaluation of collection attributes so the generated code adds a reference to a TypeDecl itself inside nestedTypes() if it was a nested type leading to infinite recursion in code generation.
    Adding @OnePhase to the collection attribute solved the problem.
    It would be preferable if the default code generation does not have this problem. I suggest using one-phase evaluation per default. What do you think? Are you in favor of one-phase evaluation or adding an extra contributor target evaluation?

  5. Jesper Öqvist

    There is a more elegant solution for this issue. Lambda functions (or anonymous class instances) could be used for computing the contributions. Instead of registering node references in the contributors set, register lambda contribution functions during the contribution phase we don’t need to evaluate either the target or the condition again.

    To illustrate my idea, I will show how it transforms the two-phase collection attribute code generation for the case I described above with nestedTypes(). Here is the JastAdd-generated code in TypeDecl without @OnePhase (simplified)

      protected void collect_contributors_TypeDecl_nestedTypes(CompilationUnit _root, java.util.Map<ASTNode, java.util.Set<ASTNode>> _map) {
        // @declaredat /home/jesper/git/extendj/java4/backend/InnerClasses.jrag:151
        if (isNestedType()) {
            TypeDecl target = enclosingType();
            java.util.Set<ASTNode> contributors = _map.get(target);
            contributors.add(this);
        }
        // @declaredat /home/jesper/git/extendj/java5/backend/EnumsCodegen.jrag:95
        {
          TypeDecl target = this;
          java.util.Set<ASTNode> contributors = _map.get(target);
          contributors.add(this);
        }
        super.collect_contributors_TypeDecl_nestedTypes(_root, _map);
      }
      /** @apilevel internal */
      protected void contributeTo_TypeDecl_nestedTypes(LinkedList<TypeDecl> collection) {
        super.contributeTo_TypeDecl_nestedTypes(collection);
        if (isNestedType()) {
          collection.add(this);
        }
        for (TypeDecl value : enumSwitchMaps()) {
          collection.add(value);
        }
      }
    

    Using lambda functions to add contributions, it would look something like this:

    public class TypeDecl {
      interface Contributor_nestedTypes {
        void contributeTo(LinkedList<TypeDecl> coll);
      }
      protected void collect_contributors_TypeDecl_nestedTypes(CompilationUnit _root, java.util.Map<ASTNode, java.util.Set<Contributor_nestedTypes>> _map) {
        // @declaredat /home/jesper/git/extendj/java4/backend/InnerClasses.jrag:151
        if (isNestedType()) {
            TypeDecl target = enclosingType();
            java.util.Set<ASTNode> contributors = _map.get(target);
            contributors.add((coll) -> { coll.add(this); });
        }
        // @declaredat /home/jesper/git/extendj/java5/backend/EnumsCodegen.jrag:95
        {
          TypeDecl target = this;
          java.util.Set<ASTNode> contributors = _map.get(target);
          contributors.add((coll) -> { coll.addAll(enumSwitchMaps()); };
      }
    }
    

    with the main collection attribute computation looking like this:

      private LinkedList<TypeDecl> nestedTypes_compute() {
        CompilationUnit root = ...;
        root.survey_TypeDecl_nestedTypes();
        LinkedList<TypeDecl> _computedValue = new LinkedList<TypeDecl>();
        if (root.contributorMap_TypeDecl_nestedTypes.containsKey(this)) {
          for (Contributor_nestedTypes contributor : root.contributorMap_TypeDecl_nestedTypes.get(this)) {
            contributor.contributeTo(_computedValue);
          }
        } 
        return _computedValue;
      } 
    

    This is a rough outline but hopefully it is clear enough to understand how it would work in practice.

  6. Log in to comment