[fpc-devel] class abstract, class sealed implementation. please review.

[Alexander Klenin]

On Sat, Oct 17, 2009 at 03:25, Paul Ishenin <webpirat at mail.ru> wrote:

> I tried to implement support for delphi sealed and abstract classes. They
> are described a bit here: http://edn.embarcadero.com/article/34324

First, let me say that I am very glad that you entered FPC development --
with your energy I hope it will see a nice progress ;-)
I too know next to nothing about FPC internals, so I have only a
generic comments:

1) (Not your fault, but) the patch is unnecessarily big
  due to msgidx.inc and msgtxt.inc changes -- why these generated files
  are under version control at all?
2) parse_object_options consumes exactly one exemplar of either
"abstract" or "sealed".
  actually, Delphi allows arbitrary number of these keywords (e.g.
"class abstract abstract").
  This is pointless, but I think should be allowed for compatibility,
perhaps with a warning.
  Also, this code would be hard to extend in case some more options are needed.
  I would suggest to trite it as a loop adding all options,
  and then after the loop check for conflicts:
     if [oo_is_abstract, oo_is_sealed] *
current_objectdef.objectoptions =  [oo_is_abstract, oo_is_sealed] then
error;
  Finally, I think the function should be named parse_class_options.
3) This patch introduces new keyword -- what about backwards
compatibility? The code like:
  type T = class sealed: Boolean; end;
will stop compiling after the patch. Maybe it is better to use another syntax?

Besides, I have some comments on the feature itself.

> At moment 'class abstract' in delphi do nothing although it should not allow
> to call a contructor of such a class. I have found QC report regards this
> but anyway parser supports them.
I see little value in this feature apart from Delphi compatibility.
Abstract classes are already identified by compiler by abstract methods.
Currently, compiler gives a warning abut them, but I think that should
actually be an error.
So, I would propose:
1) In Delphi mode, accept "abstract" keyword and warn about abstract methods.
2) In OBJFPC mode, do not accept "abstract" keyword and change warning to error.

> Sealed class is a class which can't be derived by another class. This one is
> fully supported by delphi.
Sealed classes is rather controversial feature, see e.g.
http://www.sellsbrothers.com/news/showTopic.aspx?ixTopic=411
http://weblogs.asp.net/cazzu/archive/2005/08/29/IHateSealed.aspx
.. and many other similar ramblings.
Also note that the original paper on sealing classes (in Java)
http://www.haifa.ibm.com/info/ple/papers/class.pdf
introduces slightly different semantics:
"Class sealing allows extension of classes within a package, but
prevent clients from extending such classes outside of the package."
This might actually make some sense, see e.g. TBasicChartSeries class
in TAChart package.
So, I would propose, in OBJFPC mode,
to allow inheriting from sealed class in the same unit

-- 
Alexander S. Klenin