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

Instantiation does not check overriding of constant property associations #1447

Closed
lwrage opened this issue Aug 14, 2018 · 21 comments
Closed

Instantiation does not check overriding of constant property associations #1447

lwrage opened this issue Aug 14, 2018 · 21 comments

Comments

@lwrage
Copy link
Contributor

@lwrage lwrage commented Aug 14, 2018

The instantiator should report an error if a contained property association overrides a property of an instance object that is marked as constant. Instance property associations already point to the declarative p.a. as their source. We need to check if that source is marked constant.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 30, 2019

I verified that this does seem to be checked for all the obvious cases in the declarative model (without any contained property associations).

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 30, 2019

Checking the current behavior for contained property associations. The following case is caught as an error in the declarative model:

package i1447_instance
public
	with TestPS;

	system A
	end A;

	system B
	end B;

	system C
	end C;

	system D
	end D;

	system implementation A.i
		subcomponents
			s1: system B.i;
	end A.i;

	system implementation B.i
		subcomponents
			s2: system C.i;
	end B.i;

	system implementation C.i
		subcomponents
			s3: system D.i;
		properties
			TestPS::myProp => 1 applies to s3;
	end C.i;

	system implementation D.i
		properties
			TestPS::myProp => constant 0;
	end D.i;
end i1447_instance;

But the instantiate (on A.i) action runs without producing any errors.

However, if I move the contained property association up a level to B.i as

		properties
			TestPS::myProp => 1 applies to s2.s3;

there is not an error on the declarative model.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 30, 2019

This case, where I have a contained property association replacing another contained property association, does not produce errors in the declarative model or during the instantiation of A.i:

package i1447_instance
public
	with TestPS;

	system A
	end A;

	system B
	end B;

	system C
	end C;

	system D
	end D;

	system implementation A.i
		subcomponents
			s1: system B.i;
	end A.i;

	system implementation B.i
		subcomponents
			s2: system C.i;
		properties
			TestPS::myProp => 2 applies to s2.s3;
	end B.i;

	system implementation C.i
		subcomponents
			s3: system D.i;
		properties
			TestPS::myProp => constant 1 applies to s3;
	end C.i;

	system implementation D.i
		properties
			TestPS::myProp => 0;
	end D.i;
end i1447_instance;

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 30, 2019

Case similar to the first one. Error on the declarative model, but nothing during instantiation.

package i1447_instance
public
	with TestPS;

	system A
	end A;

	system B
	end B;

	system C
	end C;

	system D
		properties
			TestPS::myProp => constant 0;
	end D;

	system implementation A.i
		subcomponents
			s1: system B.i;
	end A.i;

	system implementation B.i
		subcomponents
			s2: system C.i;
	end B.i;

	system implementation C.i
		subcomponents
			s3: system D.i;
		properties
			TestPS::myProp => 10 applies to s3;
	end C.i;

	system implementation D.i
	end D.i;
end i1447_instance;

Again, when I move the contained property association up a level to B.i there is no error on the declarative model.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 30, 2019

Trying applies to on features now.

package i1447_instance
public
	with TestPS;

	data X
	end X;
	
	data Y extends X
	end Y;

	data implementation Y.i
	end Y.i;

	data implementation Y.j extends Y.i
	end Y.j;

	system A
	end A;

	system B
	end B;

	system C
	end C;

	system D
		features
			f1: out data port Y.j {
				TestPS::myProp => constant 100;
			};
	end D;

	system implementation A.i
		subcomponents
			s1: system B.i;
	end A.i;

	system implementation B.i
		subcomponents
			s2: system C.i;
	end B.i;

	system implementation C.i
		properties
			TestPS::myProp => 200 applies to s3.f1;
	end C.i;
end i1447_instance;

There are no errors of any sort reported here even though the contained property association in C.i tries to replace the constant property association that comes from D.

Also, if I mark the contained association in C.i as constant and add the following to B.i

			TestPS::myProp => 300 applies to s2.s3.f1;

there are no errors generated anywhere, even though I should not be able to replace the value associated in C.i.

@lwrage lwrage added this to the 2.5.1 milestone May 31, 2019
@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 31, 2019

Test connections

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 31, 2019

In

	system w
	end w;

	system implementation w.i
		subcomponents
			xc: system x.i;
			yc: system y.i;
		connections
			conn: feature xc.outPort -> yc.inPort {
						TestPS::myProp => constant 0;
					};
		properties
			TestPS::myProp => 1 applies to conn;
	end w.i;

the property association on w.i has an error in the declarative model.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 31, 2019

in

	system w
	end w;

	system implementation w.i
		subcomponents
			xc: system x.i;
			yc: system y.i;
		connections
			conn: feature xc.outPort -> yc.inPort {
						TestPS::myProp => constant 0;
					};
	end w.i;

	system v
	end v;

	system implementation v.i
		subcomponents
			wc: system w.i;
		properties
			TestPS::myProp => 1 applies to wc.conn;
	end v.i;

No errors are reported even though the association in v.i should not be allowed.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 31, 2019

in

	system w
	end w;

	system implementation w.i
		subcomponents
			xc: system x.i;
			yc: system y.i;
		connections
			conn: feature xc.outPort -> yc.inPort {
						TestPS::myProp => 100;
					};
	end w.i;

	system v
	end v;

	system implementation v.i
		subcomponents
			wc: system w.i;
		properties
			TestPS::myProp => constant 10 applies to wc.conn;
	end v.i;

	system u
	end u;

	system implementation u.i
		subcomponents
			uc: system v.i;
		properties
			TestPS::myProp => 20 applies to uc.wc.conn;
	end u.i;

No errors are reported even though the association in u.i should not be allowed.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented May 31, 2019

This should be pretty easy to fix, I think.

Need to update

  • CacheContainedPropertyAssocationsSwitch.processContainedPropertyAssociations methods

Not sure what I have to do for connections. Vlaues end up in the SCProperties cache, what happens after that?

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 4, 2019

How do we want to report this error? The error is detected on the instance model at the point of the constant property associatoin, but the error itself is in the declaratie model at the point of the contained property association.

@lwrage
Copy link
Contributor Author

@lwrage lwrage commented Jun 4, 2019

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 5, 2019

Okay, this *does not * work the way I though it did. I thought contained property associations were being applied after the local property associations were already in the instance model and that the contained ones would thus overwrite them. This would mean the original prpoerty association could be checked to see if it was marked constant. This is not the case. The contained property associations are processed first for some reason.

So I think what I need to do is in CacheContainedPropertyAssocationsSwitch.processContainedPropertyAssociations() I need to lookup the existing property association, which will force it to retrieve the value from the declarative model, and check for the constant flag. I'm concerned how the modes will work in this situation.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 5, 2019

I noticed an additional issue. When creating a new property association on the instance object in CacheContainedPropertyAssocationsSwitch.processContainedPropertyAssociations() the constant flag is not copied to the new association.

Actually, I get an this from the declarative PA that is pointed to by the new instance PA.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 6, 2019

Ok, I have this working, I think, for component instances. Should be easy to extend to features. Then I have to deal with Semantic connections.

Going to create the regression test cases for component instances first.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 7, 2019

Added JUnit tests for subcomponents.

Now to fix features.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 7, 2019

Don't Forget feature groups!

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 11, 2019

Fixed feature groups

Next up, semantic connections

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 11, 2019

I think for semantic connections we need to check as we update scProps at line 347 in CacheContainedPropertyAssociations.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 13, 2019

For semantic connections:

  1. It is easy to check if a contained property association tries to replace a constant contained property association.

  2. It is not so easy to check if a contained property association tries to replace a constant property association from the declarative model because that property could come from any of the declarative connections that make up the semantic connection.

I think I should deal with 1, but not 2.

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jun 13, 2019

Actually semantic connections are just plain weird. What does it mean to have a constant property association on one of the declarative connections that comprises a semantic connection? Attempts to alter that property association on a different declarative connection will yield inconsistency errors during instantiation. Even contained property associations are still tracked with reference to the declarative connection, so errors regarding constant contained property associations will only be triggered if the same declarative connection is referenced. Otherwise, an inconsistency error will be generated.

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.

2 participants