Discussion:
[fpc-pascal] Multi-scope helpers draft
Ryan Joseph
2018-11-24 04:20:05 UTC
Permalink
I’d like to propose this mode switch ($modeswitch multiscopehelpers) to allow multiple helpers per scope. I have no idea why Delphi thinks only one helper should be allowed in any scope but it cripples the feature severely. Sharing helpers is mostly not possible because of potential conflicts and even relying on helpers in your own code base is not safe because conflicts could occur later. Helpers are basically just a way to extend procedural calls to dot notation so I don’t understand why this restriction was ever out in place (Objective-C and C# never imposed the restriction on their categories/extensions).

All of the hard work was already done so it was just a matter of lifting the artificial restriction that was placed on them. There’s still clean up to do and as always I may have misunderstood something fundamental about the compiler design.

https://github.com/genericptr/freepascal/commits/helperscope

Regards,
Ryan Joseph

_______________________________________________
fpc-pascal maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org
Sven Barth via fpc-pascal
2018-11-25 16:03:01 UTC
Permalink
Post by Ryan Joseph
I’d like to propose this mode switch ($modeswitch multiscopehelpers) to allow multiple helpers per scope. I have no idea why Delphi thinks only one helper should be allowed in any scope but it cripples the feature severely. Sharing helpers is mostly not possible because of potential conflicts and even relying on helpers in your own code base is not safe because conflicts could occur later. Helpers are basically just a way to extend procedural calls to dot notation so I don’t understand why this restriction was ever out in place (Objective-C and C# never imposed the restriction on their categories/extensions).
All of the hard work was already done so it was just a matter of lifting the artificial restriction that was placed on them. There’s still clean up to do and as always I may have misunderstood something fundamental about the compiler design.
https://github.com/genericptr/freepascal/commits/helperscope
This is a feature I'll definitely support as that was on my ToDo list
for quite some time already.

It's a good thing that you saw the errors in your first design and
rectified those in the third commit. ;) That commit however contains
unnecessary noise (new line changes? space changes?), so when reworking
the commits for a patch please try to get rid of these.

One thing that bothers me is the "lastonly" parameter. Why did you add
that? In the two locations you added them you'd now have a problem if
multiple helpers are in scope, but the last one does not contain the
requested symbol. In my opinion that parameter is not needed at all.

Another important part of course are tests, both tests that should work
and those that should not.

But other than that this looks good.

Regards,
Sven
_______________________________________________
fpc-pascal maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listi
Ryan Joseph
2018-11-26 02:11:41 UTC
Permalink
It's a good thing that you saw the errors in your first design and rectified those in the third commit. ;) That commit however contains unnecessary noise (new line changes? space changes?), so when reworking the commits for a patch please try to get rid of these.
Yeah I don’t know what Git thinks changed so I just ignored it. Maybe my text editor (Sublime Text) changed the indention from tabs to spaces or something.

It thinks these 2 lines are different for some reason (copied from git):

- if not (ocf_check_non_overloadable in ocf) and not isunaryoperatoroverloadable(t.nodetype,inlinenumber,ld) then
+ if not (ocf_check_non_overloadable in ocf) and not isunaryoperatoroverloadable(t.nodetype,inlinenumber,ld) then
One thing that bothers me is the "lastonly" parameter. Why did you add that? In the two locations you added them you'd now have a problem if multiple helpers are in scope, but the last one does not contain the requested symbol. In my opinion that parameter is not needed at all.
I was trying to reduce the exposure of my changes to the rest of the code base but I’ll remove it if you think it’s safe.

Regards,
Ryan Joseph

_______________________________________________
fpc-pascal maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mai
Sven Barth via fpc-pascal
2018-11-26 06:54:09 UTC
Permalink
Post by Sven Barth via fpc-pascal
On Nov 25, 2018, at 11:03 PM, Sven Barth via fpc-pascal <
It's a good thing that you saw the errors in your first design and
rectified those in the third commit. ;) That commit however contains
unnecessary noise (new line changes? space changes?), so when reworking the
commits for a patch please try to get rid of these.
Yeah I don’t know what Git thinks changed so I just ignored it. Maybe my
text editor (Sublime Text) changed the indention from tabs to spaces or
something.
- if not (ocf_check_non_overloadable in ocf) and not
isunaryoperatoroverloadable(t.nodetype,inlinenumber,ld) then
+ if not (ocf_check_non_overloadable in ocf) and not
isunaryoperatoroverloadable(t.nodetype,inlinenumber,ld) then
You could use an editor that shows non printable characters and check what
got changed there.
Post by Sven Barth via fpc-pascal
One thing that bothers me is the "lastonly" parameter. Why did you add
that? In the two locations you added them you'd now have a problem if
multiple helpers are in scope, but the last one does not contain the
requested symbol. In my opinion that parameter is not needed at all.
I was trying to reduce the exposure of my changes to the rest of the code
base but I’ll remove it if you think it’s safe.
That's were running the testsuite comes in. Both for existing tests to
discover regressions and new tests to ensure that everything works as
expected.
In this case you'd write a test for the situation I described, run that an
notice that it won't even compile. Thus you'd learn that the "lastonly"
parameter made things worse.

A bit more information for testing FPC can be found here if you don't have
seen that already: http://wiki.freepascal.org/Testing_FPC

Regards,
Sven
Loading...