Discussion:
ANN: Management operators - final patch
(too old to reply)
Maciej Izak
2016-11-21 01:16:58 UTC
Permalink
Raw Message
Hi,

Patch for management operators is totally reworked. All suggestions from
Florian, Jonas and Sven have been implemented. This is the third attempt.
My base is trunk r34916. Each commit works well with "make clean all". I
have new dedicated branch located here:

https://github.com/maciej-izak/freepascal/tree/fpc-management-operators

Patch has 3 parts (17 patches attached in single zip file
"mo-patch-final.zip" in bug report :
http://bugs.freepascal.org/view.php?id=30687 )

*PART 1*

Patch 1-3 minor small improvement and typo fixes for compiler:

1.
https://github.com/maciej-izak/freepascal/commit/bfabb129877c1663b8e2a9556685aef2729ac175
2.
https://github.com/maciej-izak/freepascal/commit/61fc1a967005d73e4bcd787a771e6f8fb0218983
3.
https://github.com/maciej-izak/freepascal/commit/452406872d9308dd51a609a509ee4353c905aeba

*PART 2*

Patch 4 . Independent optimization (Jonas request long time ago)
for InitializeArray/FinalizeArray . Is not directly related to management
operators.

4.
https://github.com/maciej-izak/freepascal/commit/ea23ca80630fae488990dcd4bc62ddc94b18d304

*PART 3*

Patch 5-17. Main patch totally reworked. All is presented in step by step
with small logic parts (I hope so).

5.
https://github.com/maciej-izak/freepascal/commit/25ed2a4d682392ca4a14854d5573fcc503b59ee1
6.
https://github.com/maciej-izak/freepascal/commit/9e1fd0886ab2f0bce0c629a1e67c08b8190af6a5
7.
https://github.com/maciej-izak/freepascal/commit/7e1c32be9f8376048541fc5891d765553752a2b8
8.
https://github.com/maciej-izak/freepascal/commit/c5db54a0e027714631a884503c52360a43513f01
9.
https://github.com/maciej-izak/freepascal/commit/59a099c0e172f8262c93cd4fb0b12806190d8166
10.
https://github.com/maciej-izak/freepascal/commit/afc64498ad23362a7bce3da6bc80fa549244cc01
11.
https://github.com/maciej-izak/freepascal/commit/4393cea745319acf3144584fc1b068aea1cf39b5
12.
https://github.com/maciej-izak/freepascal/commit/0b0197c9f1bfba05dfe1486f9512877b63f9c928
13.
https://github.com/maciej-izak/freepascal/commit/9baa32a49a204686427981d33c9054f64b2057c4
14.
https://github.com/maciej-izak/freepascal/commit/5043b4bffabb9091b3dc828ea0f3b7af6eaab6e6
15.
https://github.com/maciej-izak/freepascal/commit/a6d73af5ade3312669a78f93068f518aeb96b1ac
16.
https://github.com/maciej-izak/freepascal/commit/62d19301e87721cfab8040718a236e00e32a6b4d
17.
https://github.com/maciej-izak/freepascal/commit/91fbd86464e47e7f81c1255904bdf0914083764c
--
Best regards,
Maciej Izak
Paul Ishenin
2016-11-21 01:54:34 UTC
Permalink
Raw Message
21.11.2016 8:16, Maciej Izak wrote:

Overall you've made a great job and a perfect example of how to supply
patches for FPC.
Post by Maciej Izak
13.
https://github.com/maciej-izak/freepascal/commit/9baa32a49a204686427981d33c9054f64b2057c4
Use fpc_Copy for Clone and fpc_Addref for Copy is misleading.

I know Florian insisted on Copy and Clone names but still for my taste
it would be the best to have conformity between compiler and RTL.

--
Best regards,
Paul Ishenin
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Maciej Izak
2016-11-21 08:04:26 UTC
Permalink
Raw Message
Post by Paul Ishenin
Use fpc_Copy for Clone and fpc_Addref for Copy is misleading.
First of all I don't like to have Florian in opposition to this feature ;)
it is a teamwork.

Anyway I agree that this is misleading... But as was suggested by Florian
(as far as I understand):

On the language level Clone and Copy is totally new feature. Users don't
have direct access to FPC_COPY nor FPC_ADDREF. FPC_COPY and FPC_ADDREF
naming is outdated and probably renaming to FPC_CLONE and FPC_COPY is good
idea, but old convention is still in usage for backward compatibility with
old code (for "alias"/"public name" feature).

I can't do much in this matter.
--
Best regards,
Maciej Izak
Florian Klämpfl
2016-11-21 21:25:43 UTC
Permalink
Raw Message
Overall you've made a great job and a perfect example of how to supply patches for FPC.
13. https://github.com/maciej-izak/freepascal/commit/9baa32a49a204686427981d33c9054f64b2057c4
Use fpc_Copy for Clone and fpc_Addref for Copy is misleading.
Copy is only equal to addref in case of arc types. If a type is only managed like a variant, copy is
not an addref operation. So it should not be called addref. As fpc_addref is not exposed to users I
think we can handle this.
I know Florian insisted on Copy and Clone names but still for my taste it would be the best to have
conformity between compiler and RTL.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Sven Barth
2016-11-21 07:01:59 UTC
Permalink
Raw Message
Post by Maciej Izak
Hi,
Patch for management operators is totally reworked. All suggestions from
Florian, Jonas and Sven have been implemented. This is the third attempt.
My base is trunk r34916. Each commit works well with "make clean all". I
have new dedicated branch located here:

Thank you for your rework. That will definitely make things easier to
include :D

Regards,
Sven
Maciej Izak
2016-11-21 09:02:29 UTC
Permalink
Raw Message
Post by Maciej Izak
https://github.com/maciej-izak/freepascal/tree/fpc-management-operators
note: management operators are also available in version for NewPascal :

https://github.com/maciej-izak/freepascal/tree/management-operators
<https://github.com/maciej-izak/freepascal/tree/fpc-management-operators>

If someone is interested in tests, "ready to use" compiler with management
operators and latest Lazarus is available at:

http://newpascal.org
--
Best regards,
Maciej Izak
Loading...