search

eclipse / xtext

Eclipse Xtext™ is a language development framework
http://www.eclipse.org/Xtext
Eclipse Public License 2.0
766 stars 320 forks source link

GrammarConstraintProvider seems to provide incorrect constraints #2018

Open philip-iii opened 2 years ago

philip-iii commented 2 years ago

I have a somewhat strange situation. Assuming a meta-model where Elements can have any number of Comments and Annotations contained in them, e.g. :

abstract class Element {
  property comment : Comment[*] { ordered composes };
  property annotation : Annotation[*] { ordered composes };
}

Other elements inherit these properties e.g. SimpleDataType extends Element and StructuredDataType extends Element { ... } where StructuredDataType has additional properties.

The grammar utilises fragments to reuse the rules for the inherited properties, e.g.

fragment AnnotationCommentFragment returns Element:
    (comment+=Comment)*
    (annotation+=Annotation)*
;

which are then used in the respective rules:

SimpleDataType_Impl returns tdl::SimpleDataType:
    AnnotationCommentFragment
    'Type' name=Identifier
;
//...
StructuredDataType returns tdl::StructuredDataType:
    AnnotationCommentFragment
    'Type' name=Identifier
    LParen (member+=Member (',' member+=Member)*)? RParen
;

The editor works as expected. The problem occurs when trying to serialise a model, loaded from XMI for example, into the textual representation defined by the grammar.

Looking into the constraints within createSequence(ISerializationContext context, EObject obj) in BacktrackingSemanticSequencer, for some reason some of the constraints, e.g. for SimpleDataType are correct:

with the many property set to true, whereas others, e.g. for StructuredDataType for some reason are incorrect:

AnnotationCommentFragment_ConstraintFragment_StructuredDataType_StructuredDataType returns StructuredDataType: (
    comment+=Comment* 
    annotation+=Annotation? 
...

with the many property set to false. That is particularly strange as the rules are also linked in the constraints and there the cardinalities are correct. To make things even more interesting, it turns out that there is a second constraint

AnnotationCommentFragment_ConstraintFragment_StructuredDataType_StructuredDataType returns StructuredDataType: (
    comment+=Comment* 
    annotation+=Annotation* 
...

which is correct, the difference being the context - in the former case, the parent is the super class, where as in the latter (and correct) case, it is StructuredDataType. During the serialisation, apparently the former is used. Instead for SimpleDataType, both contexts (super class and subclass) are assigned to the same constraint.

Digging a bit deeper, suggests that different NDAs and potentially also corresponding PDAs are produced instead of a single one unifying the contexts for the super and subclasses.

Any ideas how that comes about and what could be done about it? I will try to put together an isolated replication kit, but there are quite a few moving parts that may be tricky to separate in a neat way..

philip-iii commented 2 years ago

OK, it turned out to be easy to isolate after all ;-) Given the following grammar (simply new project -> replace grammar):

grammar org.xtext.example.mydsl.MyDsl with org.eclipse.xtext.common.Terminals

generate myDsl "http://www.xtext.org/example/mydsl/MyDsl"

Package:
    'Package'
    name=ID
    '{'
        (packagedElement+=PackageableElement)*
    '}'
;

PackageableElement:
    (   AnnotationType
        | SimpleDataType 
        | StructuredDataType
    )   
;

AnnotationType:
    AnnotationCommentFragment
    'AnnotationType' name=ID
;

DataType:
    SimpleDataType | StructuredDataType
;

SimpleDataType:
    AnnotationCommentFragment
    'Type' name=ID
;

StructuredDataType:
    AnnotationCommentFragment
    'Type' name=ID
    '(' (member+=Member (',' member+=Member)*)? ')'
;

Member:
    AnnotationCommentFragment
    (isOptional?='optional')?
    dataType=[DataType|ID]
    name=ID 
;

Comment:
    'Note:' body=STRING
;

Annotation:
    '@' key=[AnnotationType|ID]
    (':' value=STRING)?
;

fragment AnnotationCommentFragment:
    (comment+=Comment)*
    (annotation+=Annotation)*
;

And the following example:

Package example {
    AnnotationType at1
    AnnotationType at2
    AnnotationType at3

    @at1
    @at2
    Type simple
    //this works! (SimpleDataType)

    @at1
    @at2
    Type structured (

    )
    //this does not (StructuredDataType)    
}

As a basic test, simply copy of the elements in the resource to a new resource:

    //assuming the example above is loaded as sourceResource
    XtextResourceSet resourceSet = injector.getInstance(XtextResourceSet.class);
    Resource targetResource = resourceSet.createResource(targetURI);
    targetResource.getContents().addAll(EcoreUtil.copyAll(sourceResource.getContents()));
    targetResource.save(null);

This results in:

java.lang.RuntimeException: Could not serialize StructuredDataType:
AnnotationCommentFragment.annotation violates the upper bound: It holds 2 values, but only 1 are allowed.
Semantic Object: Package'example'.packagedElement[4]->StructuredDataType'structured'
URI: platform:/resource/test/isolated/example.mydsl.mydsl
Context: PackageableElement returns StructuredDataType
    at org.eclipse.xtext.serializer.diagnostic.ISerializationDiagnostic$ExceptionThrowingAcceptor.accept(ISerializationDiagnostic.java:132)
    at org.eclipse.xtext.serializer.sequencer.BacktrackingSemanticSequencer.createSequence(BacktrackingSemanticSequencer.java:514)
    at org.xtext.example.mydsl.serializer.MyDslSemanticSequencer.sequence_AnnotationCommentFragment_StructuredDataType(MyDslSemanticSequencer.java:122)
    at org.xtext.example.mydsl.serializer.MyDslSemanticSequencer.sequence(MyDslSemanticSequencer.java:61)
    at org.eclipse.xtext.serializer.sequencer.AbstractSemanticSequencer.createSequence(AbstractSemanticSequencer.java:68)
    at org.eclipse.xtext.serializer.acceptor.SequenceFeeder.acceptEObjectRuleCall(SequenceFeeder.java:327)
    at org.eclipse.xtext.serializer.acceptor.SequenceFeeder.acceptRuleCall(SequenceFeeder.java:354)
    at org.eclipse.xtext.serializer.acceptor.SequenceFeeder.accept(SequenceFeeder.java:265)
    at org.eclipse.xtext.serializer.sequencer.BacktrackingSemanticSequencer.accept(BacktrackingSemanticSequencer.java:445)
    at org.eclipse.xtext.serializer.sequencer.BacktrackingSemanticSequencer.createSequence(BacktrackingSemanticSequencer.java:512)
    at org.xtext.example.mydsl.serializer.MyDslSemanticSequencer.sequence_Package(MyDslSemanticSequencer.java:176)
    at org.xtext.example.mydsl.serializer.MyDslSemanticSequencer.sequence(MyDslSemanticSequencer.java:53)
    at org.eclipse.xtext.serializer.sequencer.AbstractSemanticSequencer.createSequence(AbstractSemanticSequencer.java:68)
    at org.eclipse.xtext.serializer.impl.Serializer.serialize(Serializer.java:122)
    at org.eclipse.xtext.serializer.impl.Serializer.serialize(Serializer.java:135)
    at org.eclipse.xtext.serializer.impl.Serializer.serialize(Serializer.java:205)
    at org.eclipse.xtext.resource.XtextResource.doSave(XtextResource.java:387)
    at org.eclipse.emf.ecore.resource.impl.ResourceImpl.save(ResourceImpl.java:1475)
    at org.eclipse.emf.ecore.resource.impl.ResourceImpl.save(ResourceImpl.java:1044)
    at org.etsi.mts.tdl.tools.rt.ui.handlers.TranslationHandler.execute(TranslationHandler.java:126)
    at org.eclipse.ui.internal.handlers.HandlerProxy.execute(HandlerProxy.java:283)
    at org.eclipse.ui.internal.handlers.E4HandlerProxy.execute(E4HandlerProxy.java:97)
    at jdk.internal.reflect.GeneratedMethodAccessor68.invoke(Unknown Source)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:58)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:319)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invoke(InjectorImpl.java:253)
    at org.eclipse.e4.core.contexts.ContextInjectionFactory.invoke(ContextInjectionFactory.java:173)
    at org.eclipse.e4.core.commands.internal.HandlerServiceHandler.execute(HandlerServiceHandler.java:156)
    at org.eclipse.core.commands.Command.executeWithChecks(Command.java:488)
    at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:487)
    at org.eclipse.e4.core.commands.internal.HandlerServiceImpl.executeHandler(HandlerServiceImpl.java:213)
    at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.executeCommand(KeyBindingDispatcher.java:308)
    at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.press(KeyBindingDispatcher.java:584)
    at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.processKeyEvent(KeyBindingDispatcher.java:653)
    at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.filterKeySequenceBindings(KeyBindingDispatcher.java:443)
    at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher$KeyDownFilter.handleEvent(KeyBindingDispatcher.java:96)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
    at org.eclipse.swt.widgets.Display.filterEvent(Display.java:1108)
    at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4436)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1512)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1535)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1520)
    at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1549)
    at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1545)
    at org.eclipse.swt.widgets.Canvas.sendKeyEvent(Canvas.java:522)
    at org.eclipse.swt.widgets.Control.doCommandBySelector(Control.java:1085)
    at org.eclipse.swt.widgets.Display.windowProc(Display.java:6246)
    at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(Native Method)
    at org.eclipse.swt.internal.cocoa.NSResponder.interpretKeyEvents(NSResponder.java:59)
    at org.eclipse.swt.widgets.Composite.keyDown(Composite.java:606)
    at org.eclipse.swt.widgets.Display.windowProc(Display.java:6078)
    at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method)
    at org.eclipse.swt.widgets.Widget.callSuper(Widget.java:235)
    at org.eclipse.swt.widgets.Widget.windowSendEvent(Widget.java:2150)
    at org.eclipse.swt.widgets.Shell.windowSendEvent(Shell.java:2487)
    at org.eclipse.swt.widgets.Display.windowProc(Display.java:6198)
    at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method)
    at org.eclipse.swt.widgets.Display.applicationSendEvent(Display.java:5442)
    at org.eclipse.swt.widgets.Display.applicationProc(Display.java:5585)
    at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(Native Method)
    at org.eclipse.swt.internal.cocoa.NSApplication.sendEvent(NSApplication.java:117)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3834)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1157)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1046)
    at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
    at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:644)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:551)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:156)
    at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:654)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:591)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1462)
    at org.eclipse.equinox.launcher.Main.main(Main.java:1435)
cdietrich commented 2 years ago

seems to be a bug with fragments.

cdietrich commented 2 years ago

Bildschirmfoto vom 2021-11-29 18-42-42

cdietrich commented 2 years ago

the comment in generated code says

     * Contexts:
     *     PackageableElement returns StructuredDataType
     *     DataType returns StructuredDataType
     *
     * Constraint:
     *     (comment+=Comment* annotation+=Annotation? name=ID (member+=Member member+=Member*)?)
     * </pre>

AnnotationCommentFragment_StructuredDataType_StructuredDataType returns StructuredDataType: (comment+=Comment annotation+=Annotation? name=ID (member+=Member member+=Member)?);

cdietrich commented 2 years ago 
1: (null) (start)
2: dataType=[DataType|ID]
3: name=ID
4: (null) (stop)
5: comment+=Comment
6: annotation+=Annotation
7: isOptional?='optional'
  _____ 1 _____ 
 /      |    \ \ 
2   __ 5 __   6 7 
|  / /  |  \  * * 
3 2 5   6   7 
| * *  /|\  * 
4     2 6 7 
      * * | 
          2 
1: (null) (start)
2: name=ID
3: (null) (stop)
4: annotation+=Annotation
5: comment+=Comment
  __ 1 
 /  /  \ 
2  4    5 
|  ||  /|\ 
3 2 4 2 4 5 
  * * * * * 
1: (null) (start)
2: name=ID
3: (null) (stop)
4: (member+=Member )
5: ( member+=Member)
6: annotation+=Annotation
7: comment+=Comment
     _ 1 __ 
    /   |  \ 
   2    6   7 
 /  |   |  /|\ 
3   4   2 2 6 7 
   / |  * * * * 
  3  5 
  *  || 
    3 5 
    * * 
1: (null) (start)
2: name=ID
3: (null) (stop)
4: (member+=Member )
5: ( member+=Member)
6: comment+=Comment
7: annotation+=Annotation
     __ 1 _____ 
    /     \    \ 
   2       6    7 
 /  |    / |\   * 
3   4   2 6  7 
   / |  * *  || 
  3  5      2 7 
  *  ||     * * 
    3 5 
    * *

unfortunately i have no ideas how the nfa work

cdietrich commented 2 years ago

log of workflow also says 2658 [main] WARN ator.serializer.SerializerFragment2 - Skipped generating duplicate method in MyDslSemanticSequencer

cdietrich commented 2 years ago

somehow we have two constraints for 1 rule. and one is false

AnnotationCommentFragment_StructuredDataType_StructuredDataType returns StructuredDataType: (comment+=Comment* annotation+=Annotation? name=ID (member+=Member member+=Member*)?);

AnnotationCommentFragment_StructuredDataType_StructuredDataType returns StructuredDataType: (comment+=Comment* annotation+=Annotation* name=ID (member+=Member member+=Member*)?);
cdietrich commented 2 years ago

these are the two nfas we have for StructuredDataType

1: (null) (start)
2: name=ID
3: (null) (stop)
4: (member+=Member )
5: ( member+=Member)
6: annotation+=Annotation
7: comment+=Comment
     _ 1 __ 
    /   |  \ 
   2    6   7 
 /  |   |  /|\ 
3   4   2 2 6 7 
   / |  * * * * 
  3  5 
  *  || 
    3 5 
    * * 

1: (null) (start)
2: name=ID
3: (null) (stop)
4: (member+=Member )
5: ( member+=Member)
6: comment+=Comment
7: annotation+=Annotation
     __ 1 _____ 
    /     \    \ 
   2       6    7 
 /  |    / |\   * 
3   4   2 6  7 
   / |  * *  || 
  3  5      2 7 
  *  ||     * * 
    3 5 
    * *
cdietrich commented 2 years ago

workaround

Package:
    'Package'
    name=ID
    '{'
        (packagedElement+=AnnotationType|packagedElement+=SimpleDataType|packagedElement+=StructuredDataType)*
    '}'
;
cdietrich commented 2 years ago

pdaProvider.getSyntacticSequencerPDAs(grammar); already seems to have two entries for structuredDatatype

philip-iii commented 2 years ago

Thanks a lot for the detailed analysis and feedback! That's about as far as I got as well, I think separate contexts are created due to the distinct NFAs, which itself seems to be rooted in the PDAs. The workaround helps as a temporary solution, but the grammar is rather large and I start to suspect that this may impact additional places, so it is not very practical in the long term.

cdietrich commented 2 years ago

to me it looks like org.eclipse.xtext.serializer.analysis.ContextTypePDAProvider.filterByType(Pda<ISerState, RuleCall>, EClass, Map<ISerState, Integer>) is not working for the pda, but no idea why.

cdietrich commented 2 years ago 
1: start (start)
2: >>AnnotationCommentFragment
3: annotation+=Annotation
4: <<AnnotationCommentFragment
5: 'Type'
6: name=ID
7: '('
8: ')'
9: stop (stop)
10: (member+=Member )
11: ','
12: ( member+=Member)
13: comment+=Comment
        1 
        | 
      _ 2 _____ 
     /     \   \ 
    3      13   4 
    |     / |\  * 
    4    3 13 4 
    |    *  * * 
    5 
    | 
    6 
    | 
  _ 7 
 /   | 
8   10 
|  / | 
9 8  11 
  *   | 
     12 
     || 
    8 11 
    *  *

vs

1: start (start)
2: >>AnnotationCommentFragment
3: annotation+=Annotation
4: <<AnnotationCommentFragment
5: 'Type'
6: name=ID
7: '('
8: ')'
9: stop (stop)
10: (member+=Member )
11: ','
12: ( member+=Member)
13: comment+=Comment
        1 
        | 
      _ 2 _____ 
     /     \   \ 
    3      13   4 
    |     / |\  * 
    4    3 13 4 
    |    *  * * 
    5 
    | 
    6 
    | 
  _ 7 
 /   | 
8   10 
|  / | 
9 8  11 
  *   | 
     12 
     || 
    8 11 
    *  *

vs

1: start (start)
2: >>AnnotationCommentFragment
3: comment+=Comment
4: annotation+=Annotation
5: <<AnnotationCommentFragment
6: 'Type'
7: name=ID
8: '('
9: (member+=Member )
10: ','
11: ( member+=Member)
12: ')'
13: stop (stop)
          1 
          | 
          2 _______ 
         /       \ \ 
  _____ 3 _____   4 5 
 /      |      \  * * 
3   ___ 4       5 
*  /     |      * 
  4      5 
  *      | 
         6 
         | 
         7 
         | 
         8 __ 
         |   \ 
        9    12 
        | \   * 
      10  12 
       |   * 
      11 
      || 
    10 12 
     *  | 
       13
cdietrich commented 2 years ago

=> wonder how the hashcodes for the SerializerPDAs work

cdietrich commented 2 years ago

here the same for Simpledatatype

1: start (start)
2: >>AnnotationCommentFragment
3: <<AnnotationCommentFragment
4: 'Type'
5: name=ID
6: stop (stop)
7: annotation+=Annotation
8: comment+=Comment
     1 
     | 
  __ 2 
 /  /  \ 
3  7    8 
|  ||  /|\ 
4 3 7 3 7 8 
| * * * * * 
5 
| 
6 
1: start (start)
2: >>AnnotationCommentFragment
3: <<AnnotationCommentFragment
4: 'Type'
5: name=ID
6: stop (stop)
7: annotation+=Annotation
8: comment+=Comment
     1 
     | 
  __ 2 
 /  /  \ 
3  7    8 
|  ||  /|\ 
4 3 7 3 7 8 
| * * * * * 
5 
| 
6 
1: start (start)
2: >>AnnotationCommentFragment
3: comment+=Comment
4: annotation+=Annotation
5: <<AnnotationCommentFragment
6: 'Type'
7: name=ID
8: stop (stop)
     1 
     | 
     2 __ 
    /  \ \ 
   3    4 5 
 / | \  * * 
3  4  5 
*  || * 
  4 5 
  * | 
    6 
    | 
    7 
    | 
    8
cdietrich commented 2 years ago

hashCode seems same but equals not (org.eclipse.xtext.serializer.analysis.SerializerPDA.equals(Object))

followers seem different [comment+=Comment, annotation+=Annotation, <<AnnotationCommentFragment] [annotation+=Annotation, <<AnnotationCommentFragment]

both from state comment

=> need to find out why comment is missing in one case (filtered out by the filter method)