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

Can't instantiate reference to property with default value #2387

Closed
joeseibel opened this issue Jul 15, 2020 · 4 comments · Fixed by #2516
Closed

Can't instantiate reference to property with default value #2387

joeseibel opened this issue Jul 15, 2020 · 4 comments · Fixed by #2516
Assignees
Milestone

Comments

@joeseibel
Copy link
Contributor

If a property value refers to a property which has a default value, then instantiation complains that the referenced property is undefined. Example model:

property set ps is
  string_property: aadlstring applies to (all);
  string_with_default: aadlstring => "default value" applies to (all);
end ps;
package instance_bug
public
  with ps;

  system s
    properties
      ps::string_property => ps::string_with_default;
  end s;

  system implementation s.i
  end s.i;
end instance_bug;

The instance model is marked with the error Property ps::string_with_default is undefined which is obviously wrong because the value is "default value".

@lwrage lwrage added the core label Aug 4, 2020
@lwrage lwrage added this to the 2.9.1 milestone Oct 2, 2020
@AaronGreenhouse AaronGreenhouse self-assigned this Nov 6, 2020
@AaronGreenhouse
Copy link
Contributor

Problem is that the property look up mechanism doesn't actually include returning the default value. It is the responsibility of the code getting the property value to take this into account.

@AaronGreenhouse
Copy link
Contributor

Need to fix this in NamedValueImpl.evaluate(). It seems like it should be fixed in PropertyImpl.evaluate(), but that would be wrong. because that method is used to evaluate all the property association look ups. The instantiation process looks up the value of any property that is explicitly associated a property anywhere in the model tree. So if a property that has a default value is explicitly associated with a value anywhere in the model, then all nodes to which the association could apply will have the property value looked up. If PropertyImpl.evaluate() is updated to return the default value, then the default value will be explicitly associated with all these locations in the final instantiated tree. This is undesirable.

Instead we have to fix the more specific case of looking up the property value as a result of it being referenced in another property value. This brings us to NamedValueImpl.evaluate():

	public EvaluatedProperty evaluate(EvaluationContext ctx, int depth) {
		AbstractNamedValue nv = getNamedValue();
		if (depth > 50) {
			throw new InvalidModelException(ctx.getInstanceObject(),
					"Property " + ((Property) nv).getQualifiedName() + " has cyclic value");
		}
		PropertyEvaluationResult pev = nv.evaluate(ctx, depth + 1);
		List<EvaluatedProperty> evaluated = pev.getEvaluated();
		if (evaluated.isEmpty()) {
			/*
			 * Issue 2387: If this NamedValue is a reference to a property value, and the
			 * property value doesn't exist, we need to check for the property's default
			 * value. (This cannot be done in PropertyImpl.evaluate(), because it is used to
			 * broadly. See the comment in PropertyImpl.evaluate().)
			 */
			if (nv instanceof Property) {
				final PropertyExpression defaultValueExpression = ((Property) nv).getDefaultValue();
				if (defaultValueExpression != null) {
					evaluated = Collections.singletonList(defaultValueExpression.evaluate(ctx, depth));
				}
			}
			// Test it again...
			if (evaluated.isEmpty()) {
				throw new InvalidModelException(ctx.getInstanceObject(),
						"Property " + ((Property) nv).getQualifiedName() + " is undefined");
			}
		}
		return evaluated.get(0);
	}

@AaronGreenhouse
Copy link
Contributor

Needs unit test

@AaronGreenhouse
Copy link
Contributor

Added test that original bug was fixed:

property set ps is
  string_property: aadlstring applies to (all);
  string_with_default: aadlstring => "default value" applies to (all);
end ps;
package instance_bug
public
  with ps;

  system s
  	features
  		f: in event port {
  			ps::string_property => "bob";
  		};
    properties
      ps::string_property => ps::string_with_default;
  end s;

  system implementation s.i
  end s.i;


  system s11
     modes
     	m1: initial mode;
     	m2: mode; 
    properties
 	  -- don't want this to force the explicit binding of the default
 	  -- value to the property in the subcomponent s2 below
       ps::string_with_default => "XXX";
  end s11;

  system s22
  	features
  		f1: in event port;
  		f2: out event port;
  end s22;


  system X
  end X;
  
  system implementation X.i
  	subcomponents
  		s1: system s11;
  		s2: system s22;
  end X.i;
end instance_bug;

Here we check that the instance model associates the value "default value" with the property string_property in the root of the instance model for s.i. We also verify that the non-default value case still works on the feature f.

We have a second test to make sure the default value for string_with_default does not get explicitly copied all over the model just because the property is explicitly assigned a value in the declarative model.

In the instance tree created from X.i, the property string_with_default should be associated with XXX on the root node. It should have no other explicit property associations.

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

Successfully merging a pull request may close this issue.

3 participants