Skip to content
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

Error on data access in the middle of end to end flow #1124

Closed
joeseibel opened this issue Apr 10, 2018 · 6 comments
Closed

Error on data access in the middle of end to end flow #1124

joeseibel opened this issue Apr 10, 2018 · 6 comments

Comments

@joeseibel
Copy link
Contributor

@joeseibel joeseibel commented Apr 10, 2018

When an end to end flow refers to a data access, an error is reported complaining that data access references are only permitted at the beginning or end of an end to end flow. This validation doesn't make sense since it can't be found in the standard. The following model should be legal:

package connect_to_subcomponent
public
  system s1
  end s1;

  system implementation s1.i1
    subcomponents
      sub1: data;
      sub2: system s2.i1;
    connections
      conn1: data access sub2.f1 <-> sub1;
  end s1.i1;

  system s2
    features
      f1: requires data access;
  end s2;

  system implementation s2.i1
    subcomponents
      sub3: abstract a1;
      sub4: abstract a2;
    connections
      conn2: data access sub3.f2 -> f1;
      conn3: data access f1 -> sub4.f3;
    flows
      --Error: Illegal reference to 'f1'.  Cannot refer to a data access except for the first and last segment of an end-to-end flow.
      etef1: end to end flow sub3.fsource1 -> conn2 -> f1 -> conn3 -> sub4.fsink1;
  end s2.i1;

  abstract a1
    features
      f2: requires data access;
    flows
      fsource1: flow source f2;
  end a1;

  abstract a2
    features
      f3: requires data access;
    flows
      fsink1: flow sink f3;
  end a2;
end connect_to_subcomponent;
@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Aug 15, 2019

This seems like a weird model. Why are you communicating between two sibling components via the parent's feature? How did this model come about?

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Aug 15, 2019

I have to agree that this situation seems is allowed by the text of the AADL spec (see below). The question is whether it is intended to be allowed.

At first I was thinking that this scenario is nonsense, because the semantics of access connections are a bit confusing, and because I keep thinking that requires access means "output" when it really means an expectation that a data object is to be provided.

If I understand things correctly, what we have is an end to end flow that describes the fact that

  1. subcomponent sub3 is pushing data into the data component that is provided to feature f1, and
  2. that data is then pulled out of that provided data component by subcomponent sub4.

This scenario would be clearer (and look less like nonsense) if a data subcomponent of s2.i1 were used instead of a data access feature. But it would still mean the same basic thing, tracking the flow of data through a "data store". The AADL standard clearly allows this based on Section 10.3 rule (N3):

The subcomponent flow identifier of an end-to-end flow declaration must name an optional flow specification in the component type of the named subcomponent or to a data component in the form of a data subcomponent, provides data access, or requires data access.

Furthermore no restrictions on the location of the data access within in the end to end flow are given.

What is strange is that the place in the verifier that is producing the error messages (Aadl2JavaValidator.typeCheckEndToEndFlowSegments()) claims to be enforcing rule (N3) above.

So in summary, so far I am convinced the checker is being too strict and the above example makes semantic sense. But I want to hear from Lutz or Peter if there is a reason this check was strengthened.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Aug 15, 2019

(Sent e-mail to Peter and Lutz and Joe about this.)

@lwrage
Copy link
Contributor

@lwrage lwrage commented Aug 16, 2019

The validator is really too strict, it is a bug.

BTW, don't expect to be able to instantiate flows that pass through data access connections, that's #643 (and a couple of related ones).

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Aug 16, 2019

Aadl2JavaValidator.typeCheckEndToEndFlowSegments() was

	private void typeCheckEndToEndFlowSegments(EndToEndFlow flow) {
		for (int i = 0; i < flow.getOwnedEndToEndFlowSegments().size(); i++) {
			EndToEndFlowSegment segment = flow.getOwnedEndToEndFlowSegments().get(i);
			if ((segment.getContext() == null || !segment.getContext().eIsProxy()) && segment.getFlowElement() != null
					&& !segment.getFlowElement().eIsProxy()) {
				if (i % 2 == 0) {
					// Checking ETESubcomponentFlow
					if (segment.getContext() == null) {
						if (segment.getFlowElement() instanceof Connection) {
							error(segment, "Illegal reference to connection '" + segment.getFlowElement().getName()
									+ "'.  Expecting subcomponent flow or end-to-end flow reference.");
						} else if (segment.getFlowElement() instanceof FlowSpecification) {
							error(segment, "Illegal reference to '" + segment.getFlowElement().getName()
									+ "'.  Cannot refer to a flow specification in the local classifier's namespace.");
						} else if (segment.getFlowElement() instanceof DataAccess && i > 0
								&& i < flow.getOwnedEndToEndFlowSegments().size() - 1) {
							error(segment, "Illegal reference to '" + segment.getFlowElement().getName()
									+ "'.  Cannot refer to a data access except for the first and last segment of an end-to-end flow.");
						}
					} else if (segment.getContext() instanceof Subcomponent) {
						if (!(segment.getFlowElement() instanceof FlowSpecification)) {
							error(StringExtensions.toFirstUpper(
									getEClassDisplayNameWithIndefiniteArticle(segment.getFlowElement().eClass()))
									+ " in " + getEClassDisplayNameWithIndefiniteArticle(segment.getContext().eClass())
									+ " is not a valid subcomponent flow.", segment,
									Aadl2Package.eINSTANCE.getEndToEndFlowSegment_FlowElement());
						}
					} else {
						error("Anything in " + getEClassDisplayNameWithIndefiniteArticle(segment.getContext().eClass())
								+ " is not a valid subcomponent flow.", segment,
								Aadl2Package.eINSTANCE.getEndToEndFlowSegment_Context());
					}
				} else {
					// Checking ETEConnectionFlow
					// Because of the parser rule ETEConnectionFlow, we know
					// that the segment.getContext() is null.
					if (!(segment.getFlowElement() instanceof Connection)) {
						error(segment, "Expected Connection, found "
								+ getEClassDisplayName(segment.getFlowElement().eClass()) + '.');
					}
				}
			}
		}
	}

I removed the unnecessary check regarding DataAccess elements:

	private void typeCheckEndToEndFlowSegments(EndToEndFlow flow) {
		for (int i = 0; i < flow.getOwnedEndToEndFlowSegments().size(); i++) {
			EndToEndFlowSegment segment = flow.getOwnedEndToEndFlowSegments().get(i);
			if ((segment.getContext() == null || !segment.getContext().eIsProxy()) && segment.getFlowElement() != null
					&& !segment.getFlowElement().eIsProxy()) {
				if (i % 2 == 0) {
					// Checking ETESubcomponentFlow
					if (segment.getContext() == null) {
						if (segment.getFlowElement() instanceof Connection) {
							error(segment, "Illegal reference to connection '" + segment.getFlowElement().getName()
									+ "'.  Expecting subcomponent flow or end-to-end flow reference.");
						} else if (segment.getFlowElement() instanceof FlowSpecification) {
							error(segment, "Illegal reference to '" + segment.getFlowElement().getName()
									+ "'.  Cannot refer to a flow specification in the local classifier's namespace.");
						}
					} else if (segment.getContext() instanceof Subcomponent) {
						if (!(segment.getFlowElement() instanceof FlowSpecification)) {
							error(StringExtensions.toFirstUpper(
									getEClassDisplayNameWithIndefiniteArticle(segment.getFlowElement().eClass()))
									+ " in " + getEClassDisplayNameWithIndefiniteArticle(segment.getContext().eClass())
									+ " is not a valid subcomponent flow.", segment,
									Aadl2Package.eINSTANCE.getEndToEndFlowSegment_FlowElement());
						}
					} else {
						error("Anything in " + getEClassDisplayNameWithIndefiniteArticle(segment.getContext().eClass())
								+ " is not a valid subcomponent flow.", segment,
								Aadl2Package.eINSTANCE.getEndToEndFlowSegment_Context());
					}
				} else {
					// Checking ETEConnectionFlow
					// Because of the parser rule ETEConnectionFlow, we know
					// that the segment.getContext() is null.
					if (!(segment.getFlowElement() instanceof Connection)) {
						error(segment, "Expected Connection, found "
								+ getEClassDisplayName(segment.getFlowElement().eClass()) + '.');
					}
				}
			}
		}
	}

@lwrage
Copy link
Contributor

@lwrage lwrage commented Aug 28, 2019

Closed by PR #1950

@lwrage lwrage closed this Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants