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

Clarify final for classes. #2719

Merged
merged 12 commits into from
Nov 25, 2020
Merged

Clarify final for classes. #2719

merged 12 commits into from
Nov 25, 2020

Conversation

HansOlsson
Copy link
Collaborator

Closes #2676
(I couldn't easily add @d-hedberg as reviewer, since not part of project.)

@henrikt-ma
Copy link
Collaborator

(I couldn't easily add @d-hedberg as reviewer, since not part of project.)

OK, I've asked him to leave a comment.

Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so now we have a pretty extensive example, but it makes me wonder if adding and adding to the example was really the right approach for defining what final means for a class.

Shouldn't the example just show what has been described in the normative text? The example suggests that there could be at least two missing parts in the normative text:

  • Inheritance.
  • Redeclaration.

(Still, I'd like someone with more experience of modeling with final classes to provide a review with approval.)

chapters/inheritance.tex Outdated Show resolved Hide resolved
chapters/inheritance.tex Outdated Show resolved Hide resolved
chapters/inheritance.tex Outdated Show resolved Hide resolved
HansOlsson and others added 2 commits November 23, 2020 08:16
Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
@HansOlsson
Copy link
Collaborator Author

OK, so now we have a pretty extensive example, but it makes me wonder if adding and adding to the example was really the right approach for defining what final means for a class.

True. But the new definition is "no modifiers for elements when using the class"; and all the example follow that.
However, that could be clearer.

@henrikt-ma
Copy link
Collaborator

OK, so now we have a pretty extensive example, but it makes me wonder if adding and adding to the example was really the right approach for defining what final means for a class.

True. But the new definition is "no modifiers for elements when using the class"; and all the example follow that.
However, that could be clearer.

The problem is that "using" is too vague when it comes to the things which could be considered indirect uses (things that are possible in presence of inheritance and redeclaration).

@henrikt-ma
Copy link
Collaborator

Since final is such a library developer oriented feature (providing no other functionality than making some models invalid, that would have worked fine if it wasn't for the violations of final), can't we ask some MAP-Lib people to join the discussion?

(I'm asking this because I wish I was in a position where I would feel confident coming with concrete proposals for what the design should be, instead of just pointing out holes in the proposed specification.)

@HansOlsson
Copy link
Collaborator Author

Since final is such a library developer oriented feature (providing no other functionality than making some models invalid, that would have worked fine if it wasn't for the violations of final), can't we ask some MAP-Lib people to join the discussion?

Regarding MAP-Lib, I opened a PR, modelica/ModelicaStandardLibrary#3649 to remove the final from final redeclare - no real comments yet.

@HansOlsson
Copy link
Collaborator Author

The problem is that "using" is too vague when it comes to the things which could be considered indirect uses (things that are possible in presence of inheritance and redeclaration).

I agree - and I tried to reformulate it to say that all elements are seen final and include that in the example; and also remove some of the odd examples as they seemed less important.

Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new approach in terms of final elements looks promising.

chapters/inheritance.tex Outdated Show resolved Hide resolved
chapters/inheritance.tex Outdated Show resolved Hide resolved
chapters/inheritance.tex Outdated Show resolved Hide resolved
@henrikt-ma
Copy link
Collaborator

henrikt-ma commented Nov 24, 2020

Regarding MAP-Lib, I opened a PR, modelica/ModelicaStandardLibrary#3649 to remove the final from final redeclare - no real comments yet.

So you mean that final redeclare would be the same as redeclare final, which should be defined as the class becoming final just in the same sense as for other final classes (and possibly clarify that the final is not what constrains further redeclaration)?

@henrikt-ma
Copy link
Collaborator

henrikt-ma commented Nov 25, 2020

Regarding MAP-Lib, I opened a PR, modelica/ModelicaStandardLibrary#3649 to remove the final from final redeclare - no real comments yet.

So you mean that final redeclare would be the same as redeclare final, which should be defined as the class becoming final just in the same sense as for other final classes (and possibly clarify that the final is not what constrains further redeclaration)?

Giving some more thought to final and replaceable, I realize that a definition only based on (recursively) treating elements as final doesn't capture the following semantics for replaceable that I – with my intuition based on rather limited experience – would have expected:

model FinalReplaceable
  import A = Modelica.Electrical.Analog;
  model M
    replaceable model R = A.Basic.Resistor constrainedby A.Interfaces.OnePort;
    R r;
  end M;

  final model FM1 = M;
  model MErr = FM1(redeclare model R = A.Basic.Conductor); // Error; Elements of final class are not replaceable.
  final model FM2 = M(redeclare model R = A.Basic.Conductor); // OK; M is not final.
  final replaceable model Port = A.Interfaces.OnePort; // Legal, but 'redeclare' is pointless.
end FinalReplaceable;

To capture this one could write:

Declaring a class final makes all its elements final, and makes the class non-replaceable. This definition applies recursively to the elements of the class that are also classes. It follows that a final class can only be used without modification.

@HansOlsson
Copy link
Collaborator Author

Regarding MAP-Lib, I opened a PR, modelica/ModelicaStandardLibrary#3649 to remove the final from final redeclare - no real comments yet.

So you mean that final redeclare would be the same as redeclare final, which should be defined as the class becoming final just in the same sense as for other final classes (and possibly clarify that the final is not what constrains further redeclaration)?

The grammar only allows 'redeclare final'; but my assumption is that some wanted to make the 'redeclare' more final by adding on a 'final' - or was influenced by other languages without 'redeclare' where instead 'final' is used to prevent further overriding of virtual function; and both were added - like "belt and suspenders".

HansOlsson and others added 4 commits November 25, 2020 09:24
Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
@HansOlsson
Copy link
Collaborator Author

I think everything should be done now.

Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me now: The normative text states the rules, and the example is only there for illustration. I'll ask @d-hedberg once more for final comments.

@d-hedberg
Copy link

Look good to me. The only change I would propose is to remove the line reading:
"This definition applies recursively to the elements of the class that are also classes."

That line emphasizes class elements. If I understand you correctly, the intent was to make it clear that all elements, including class elements are made recursively final. However, that statement can also be a source of confusion, making the reader wonder if only class elements are recursively made final, and not component elements.

@HansOlsson
Copy link
Collaborator Author

Look good to me. The only change I would propose is to remove the line reading:
"This definition applies recursively to the elements of the class that are also classes."

That line emphasizes class elements. If I understand you correctly, the intent was to make it clear that all elements, including class elements are made recursively final. However, that statement can also be a source of confusion, making the reader wonder if only class elements are recursively made final, and not component elements.

Well remove, or simplify to ""This definition applies recursively to the elements of the class."
Which one do you prefer?

@d-hedberg
Copy link

d-hedberg commented Nov 25, 2020 via email

@henrikt-ma
Copy link
Collaborator

Look good to me. The only change I would propose is to remove the line reading:
"This definition applies recursively to the elements of the class that are also classes."

That line emphasizes class elements. If I understand you correctly, the intent was to make it clear that all elements, including class elements are made recursively final. However, that statement can also be a source of confusion, making the reader wonder if only class elements are recursively made final, and not component elements.

OK, I think you didn't read it as intended. The sentence speaks only of the class elements since it is only for class elements it makes sense to apply the rule recursively. For non-class elements (component declarations), this rule has nothing to say in addition to just treat them as final components, which is defined in the first paragraph of the section…

Now, when I read that paragraph together with the paragraph added by this PR, I'm actually unable to see that the new normative paragraph is adding. Which part of the new rule doesn't follow by this general rule:

An element defined as final by the final prefix in an element modification or declaration cannot be modified by a modification or by a redeclaration. All elements of a final element are also final.

I'm afraid we've been going in circles, and that the only part we should keep is the new example… possibly with some added text to the two examples saying that the first is Final modification, and the second is Final class declaration?

@HansOlsson
Copy link
Collaborator Author

I'm afraid we've been going in circles, and that the only part we should keep is the new example… possibly with some added text to the two examples saying that the first is Final modification, and the second is Final class declaration?

That would also work for me; assuming the first is "Final component modification"; as the distinction is more between component and class.

@henrikt-ma
Copy link
Collaborator

I'm afraid we've been going in circles, and that the only part we should keep is the new example… possibly with some added text to the two examples saying that the first is Final modification, and the second is Final class declaration?

That would also work for me; assuming the first is "Final component modification"; as the distinction is more between component and class.

Sounds good to me.

@HansOlsson
Copy link
Collaborator Author

Sounds good to me.

Done

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

Successfully merging this pull request may close these issues.

final or not final?
3 participants