Multi-scope helpers draft

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Multi-scope helpers draft

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

Regards,
        Ryan Joseph

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Multi-scope helpers draft

Free Pascal - General mailing list
Am 24.11.2018 um 05:20 schrieb 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  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Multi-scope helpers draft

Ryan Joseph


> On Nov 25, 2018, at 11:03 PM, Sven Barth via fpc-pascal <[hidden email]> wrote:
>
> 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  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Multi-scope helpers draft

Free Pascal - General mailing list
Am Mo., 26. Nov. 2018, 03:12 hat Ryan Joseph <[hidden email]> geschrieben:


> On Nov 25, 2018, at 11:03 PM, Sven Barth via fpc-pascal <[hidden email]> wrote:
>
> 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

You could use an editor that shows non printable characters and check what got changed there. 


> 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 

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Multi-scope helpers draft

Free Pascal - General mailing list
In reply to this post by Ryan Joseph
On Sat, 24 Nov 2018 11:20:05 +0700
Ryan Joseph <[hidden email]> wrote:

> I’d like to propose this mode switch ($modeswitch multiscopehelpers)
> to allow multiple helpers per scope.

Can you explain the order/precedence of multiple helpers?

Btw, why is the modeswitch called multiscopehelpers instead of
multihelpers or allhelpers?

Mattias
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal