New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

a wrong warnning for two events in a state machine with the same name but different parameters #656

Closed
vahdat-ab opened this Issue Nov 26, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@vahdat-ab
Member

vahdat-ab commented Nov 26, 2015

If the following example is executed,

<//----- Example -----
class A{
    status {
        on{
            turnOff(Integer i) -> off;
            turnOff -> off;         
        }
        off{
            turnOn -> on;   
        }
    }
}
//----- End ----->

There is a warning saying

Warning 54 on line 5 of file "test.ump":
Transition turnOff will be ignored due to a unguarded duplicate event

This is not valid because the names are the same, but the parameters are different. Therefore, the must be a valid statemenet.

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Dec 9, 2015

another issue from the same source

class A {isA T1;} 
trait T1 { 
    status { 
        on { 
            turnOff(int t)-> off;
            turnOn -> on;
        } 
        off{
            turnOff->off;
            turnon -> on;
        }
    }
}

In the generated code, there must be two events turnOff. One with an int parameter and anther one without a parameter.

@nwam

This comment has been minimized.

Contributor

nwam commented Jan 20, 2017

Even after deleting all the occurrences of warning 54, the generated code does not contain overloaded methods. This error is raised twice, ever, and it is in cruise.umple/src/UmplerInternalParser_CodeStateMachine.ump :

    if(priorAutoTransitionExists){
      setFailedPosition(subToken.getPosition(), 54, subToken.getValue("event"));
      ignoredTransitions.add(subToken);
    }
    if(unguardedEvents.contains(subToken.getValue("event"))){
      setFailedPosition(subToken.getPosition(), 54, subToken.getValue("event"));
      ignoredTransitions.add(subToken);
    }

I believe that this problem persists after removing these lines is due to the means of which events are retrieved or created in the analyzeTransition and analyzeStandAloneTransition methods in the same file; they use getEvent(String s) and findOrCreateEvent(eventName) which can only retrieve the same event on every call.

I will continue to investigate this.

@nwam

This comment has been minimized.

Contributor

nwam commented Jan 21, 2017

After setting up UmpleInternalParser_CodeStateMachine.ump to search and create Events by both name and parameters, I hit another roadblock. The test eventsWithInconsistentArguments is now failing because it is not throwing an error for having two events with the same name, but different arguments. This makes sense because Events are now set up to be able to be able to be overloaded.

This is fine for languages like Java and C++, however for languages like PHP, where overloading is not available, this will not do.

@nwam

This comment has been minimized.

Contributor

nwam commented Jan 21, 2017

Aside from my previous comment, in my fork, the generated (Java) code for the examples at the top of this page still do not include overloaded methods. Only the first Event will generate a method. For example, when generating the first example on this page, turnOff(Integer i) gets generated, and turnOff() does not.

I still have not replaced all occurrences of findOrCreateEvent(String) with the new findOrCreateEvent(String, List<MethodParameter>) as this gets pretty difficult in some files where parameter data is not immediately available, but that is the suspected cause of this issue.

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Jan 22, 2017

After all, I think we made a decision long ago that having events with the same name (one with no arguments and one/some with arguments) should not be allowed because a) We could never make it work in PhP (although state machines are not supported in php); b) It probably provides little or no benefit. So after all this work, I think we ought to abandon and close this issue.

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Jan 22, 2017

Ok, I'm closing this.

@vahdat-ab vahdat-ab closed this Jan 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment