Provide an option to mark a TdwsClass as "internal"
Proposal
I propose that TdwsClass
be extended with a property that marks the class as being "internal".
An internal class:
- Cannot be overridden from script.
- Cannot be instantiated from script.
Background
Let's say I have a class TMyStuff
, declared in a TdwsUnit
, that wraps a Delphi side object. I also have a function that returns an instance of this class: function GetStuff: TMyStuff
.
The class can only be instantiated through this function and thus has no constructor.
Now in order to prevent my users from instantiating the class via the TObject
constructor I can declare the class abstract and sealed:
type
TMyStuff = class abstract sealed
end;
If the user tries to instantiate via script they get a compile time error:
Error: Trying to create an instance of an abstract class
This works, inasmuch as it prevents the user from instantiating the class via the TObject
constructor, but is a bit misleading since the class is in fact not abstract.
The problem with the above solution is that it requires that I declare the base class sealed.
If I need to declare another class TMyOtherStuff
derived from TMyStuff
then TMyStuff
can no longer be declared sealed:
type
TMyStuff = class abstract
end;
TMyOtherStuff = class abstract sealed(TMyStuff)
end;
...and it is now possible for the user to instantiate an class that was never meant to be instantiated from script. Even though I declare TMyOtherStuff
abstract and sealed:
type
TMyUserStuff = class(TMyStuff);
var Stuff := TMyUserStuff.Create;
The above compiles without any warning or errors but as soon as the user calls any method on the object it fails with an access violation since the object wasn't instantiated correctly (the internal ExternalObject pointer was never assigned a value).
The only solution I have found to the above problem is to add a constructor to the base class and raise an exception in this constructor if the user tries to use it (i.e. instantiate the object from script). This works but is a really poor solution because:
- It requires us to write these do-nothing constructors for every class that the restrictions applies to.
- It causes a run time error instead of a compile time error.
- It generates confusion among the users which in turn leads to an increase in support.
Comments (11)
-
repo owner -
repo owner - changed status to resolved
-
reporter - changed status to open
- edited description
Although using
Object
as the ancestor class would indeed remove the defaultTObject
constructor, this is easily circumvented by the user:type TMyUserStuff = class(TMyStuff) constructor Create; begin end; end; var Crash := TMyUserStuff.Create; Crash.SomeMethod; // Causes run-time Access Violation
It also introduces the burden that I now have to write explicit constructors for all internal classes - even though most of them will be completely empty.
-
repo owner You can prevent that by marking your classes as class 'sealed'.
Also why do you need an empty constructor for your internal classes?
-
reporter Going back to my original example, specifying
Object
as the base class, the TdwsUnit declaration now looks like this:type TMyStuff = class abstract (Object) // Cannot be marked sealed since it's an abstract base class end; TMyOtherStuff = class abstract sealed(TMyStuff) end;
This means that although the user cannot directly create an instance of
TMyStuff
orTMyOtherStuff
, the user can still derive fromTMyStuff
and add their own constructor.
Deriving from
Object
I need to write explicit constructors since the defaultTObject
is no longer available Delphi side.Let's say I have a TdwsUnit function that returns a
TMyStuff
object. Deriving fromTObject
I can return an instance of the object like this:Info.ResultAsVariant := Info.Vars[Info.ResultVars.TypeSym.Name].GetConstructor('Create', MyExternalObject).Call.Value;
and I don't need to implement a explicit constructor.
Deriving from
Object
I no longer have the defaultCreate
constructor, soGetConstructor('Create',...)
doesn't work unless I explicitly implement that constructor. -
repo owner Calling the script-side 'Create' constructor is only required if you want it to be executed, in your case, since you have none, it would just be a long way to create the script object instance and do nothing on it. I also guess your external object does not have any script-side fields?
var obj : TScriptObjInstance; ... obj := TScriptObjInstance.Create(info.Table.FindTypeSymbol('TMyStuff', cvMagic) as TClassSymbol); obj.ExternalObject := MyExternalObject; Info.ResultAsVariant := IScriptObj(obj);
This will directly create the script object and assign it as result.
In a practical implementation you would probably store the class symbol to avoid looking it up by name every time like in the snippet above.
-
reporter Excellent! I must have missed that little detail in the documentation :-)
For my use I had to also pass Info.Execution to the
TScriptObjInstance
constructor because I need it set before returning from the method (because I need access to Execution.Environment).At this time I don't think I have a single case where I need to call the script side constructor from the Delphi side, so this will make a big difference. It also relieves me of passing
IInfo
around when all I really need isIScriptObj
.W.r.t. saving a reference to the symbol I guess the place to store that would be in the Environment, right?
-
reporter - changed status to resolved
-
repo owner The reference to the symbol can be saved in the environment, but the environment is related to a script execution, while the symbol is related to a script compilation (IdwsProgram).
There is not really any place to reference that kind of things from a script program at the moment.
So far in my use cases I usually have some kind of structure above the script programs (typically what needs/uses the script), and I place that kind of data there. Since it's also the structure that will trigger a compilation, it can just lookup the symbols it needs after compiling. You can then "forward" those symbols through the environment (f.i. through OnExecutionStarted event, or just attaching it directly after a CreateNewExecution), another possibility would be using the (relatively) new CustomStates mechanism (where the environment is meant for execution-oriented data, custom states are meant for library-oriented data).
-
reporter - changed status to open
I have finally had an opportunity to try out the solution you outlined (using
Object
as the base class) and I'm afraid it doesn't work.I have the following class hierarchy declared in TdwsUnit:
TPersistent = class abstract(Object) protected procedure AssignTo(Dest: TPersistent); virtual; public procedure Assign(Source. TPersistent); virtual; // No constructor, nor destructor declared here end; TStrings = class abstract(TPersistent) [... 43 methods - not relevant to problem ...] // No constructor, nor destructor declared here end;
Now if I try to instantiate a
TPersistent
orTStrings
in script:var Strings := TStrings.Create;
then I correctly get an error at compile time: Syntax Error: There is no accessible member with name "Create"
And if I derive a class from
TStrings
I get the same error:type TMyStrings = class(TStrings); var Strings := TStrings.Create;
However if I add a constructor then I'm allowed to compile...
type TMyStrings = class(TStrings) public constructor Create; begin end; end; var Strings := TMyStrings.Create; Strings.Add('test'); // Access Violation at run time
... but the script naturally bombs at runtime with an AV in the call to
TStrings.Add
since no Delphi side object has been instantiated.Here's my implementation of
TStrings.Add
:procedure TDataModuleScriptClasses.dwsUnitClassesTStringsMethodsAddEval(Info: TProgramInfo; ExtObject: TObject); begin Info.ResultAsInteger := TStrings(ExtObject).Add(Info.Params[0].ValueAsString); end;
In the above
ExtObject
isnil
.As a bonus I also get an error upon destruction of the
TMyStrings
instance: Runtime Error: TPersistent.AssignTo(nil) not allowed. Not that it's relevant to the problem but here's my implementation ofTPersistent.AssignTo()
:procedure TDataModuleScriptClasses.dwsUnitClassesClassesTPersistentMethodsAssignToEval(Info: TProgramInfo; ExtObject: TObject); begin if (Info.Params[0].ScriptObj = nil) then raise Exception.Create('TPersistent.AssignTo(nil) not allowed'); TPersistentCracker(ExtObject).AssignTo(TPersistent(Info.ParamAsObject[0])); end;
My guess is that there's either an incorrect cast to
TObject
somewhere in the codegen or that the codegen assumes the first class method to be the destructor.
Unless I've made a mistake in the above I'd like to reissue my initial proposal for an Internal flag.
-
repo owner - changed status to resolved
Fixed issue
#83, support for internal classes and internal execution flags→ <<cset b10f1c65a990>>
- Log in to comment
Committed a couple of fixes that allow using "Object" as ancestor rather than "TObject", this had been added a while ago (https://www.delphitools.info/2012/10/03/re-rooting-object-pascal/) but was not fully supported yet for TdwsUnit classes.
If your classes inherit from "Object", they will have none of the default methods (constructors, destructors, etc.), so you can choose to introduce whatever capability you want wherever you want.