Recent

Author Topic: Type checking of typed pointers, option -Sy, directive {$T+}  (Read 3374 times)

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4571
  • I like bugs.
The development version of Lazarus IDE now has "@<pointer> returns a typed pointer (-Sy, {$T+})" in Project Compiler Options, Parsing pane.
Please test and set it as default for new projects using the "Set compiler options as default" Checkbox at bottom left of the Project Options window.

Typed pointers do not have type checking by default when using the address @ syntax! It means the following code compiles :
Code: Pascal  [Select][+][-]
  1. var
  2.   I : Integer;
  3.   P : PChar;
  4. begin
  5.   P:=@I;
  6. end.
(That snippet is from FPC documentation.)
It feels almost like a bug in a strongly types language. -Sy or {$T+} brings type checking there. The above snippet fails to compile with -Sy, as it should.

-Sy finds real errors, for example :
 https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/40265
Sometimes however valid code fails to compile and a fix makes it longer and harder to read. An example from a patch in issue :
 https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/40263
Code: Pascal  [Select][+][-]
  1. -    Polygon(DC, @Points, 4, False);
  2. +    Polygon(DC, @Points[Low(Points)], 4, False);
"Points" is an array. Polygon() needs an address for its first element. "@Points" gives address for the first element but -Sy demands it explicitly with "@Points[Low(Points)]". It repeats the word "Points" and does not look as nice.
Is there a way to improve this?
Adding a Pointer() typecast makes everything compile but that is dummy and does not guarantee valid code. Then it is better not to use -Sy at all.

I hope "Arioch The" joins the forum discussion. He writes long endless rants in the bug tracker which is a wrong place for it.

It would be nice to make some packages / libraries compile with -Sy. LazUtils at least.
« Last Edit: May 16, 2023, 05:27:10 pm by JuhaManninen »
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 11338
  • Debugger - SynEdit - and more
    • wiki
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #1 on: May 17, 2023, 02:35:26 pm »
I guess this also affects pointer arithmetic.

E.g.
Code: Pascal  [Select][+][-]
  1. for i := 0 to sizeof(MyQWord)-1 do
  2.   MySum := MySum + PByte( @MyQWord+i )^;

Will without typed pointer, add all the bytes in the QWord.
But with typed pointer it will  add 8 times i bytes to the address (inc to the next qword). And then take the first byte of each of those qwords.

Both will compile. Just the result will be different.

Bart

  • Hero Member
  • *****
  • Posts: 5573
    • Bart en Mariska's Webstek
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #2 on: May 19, 2023, 10:31:01 am »
Why not add {$T-} to those units that depend on not being compiled with typed pointers?
(As long as current implementation, wiht typed pointers off, of course is correct.)

Bart

Arioch

  • Sr. Member
  • ****
  • Posts: 421
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #3 on: May 19, 2023, 07:47:33 pm »
Sorry, i am really short of time there days.

Few changes i made that day while hoping it would be a low hanging fruit are https://gitlab.com/ariochthe/source/-/commits/win32api_fix

I wish the compiler added some help by https://gitlab.com/freepascal.org/fpc/source/-/issues/40275 and by extending the DLL "delayed loading", but i appreciate everyone is busy these days as exonomics goes down everywhere.

I would try to keep an eye, but i would not probably be active.

Would this be a low-hanging fruit... but it would be more like rolling Sisyphus stone againt FPC team and LCL team established practices. More duplication of code and efforts like at https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39935

Why?..

One day BPL's would arrive and Lazarus would no more need FPC RTL (as sources) and we can have another look at this, perhaps.

----

> Why not add {$T-} to those units

Like i said in the tracker - it will destroy the very idea. This switch NOW has no big meaning - users for years were testing the implementation.

This switch has two merits:

1. helps to write FUTURE code with more safety
2. highlights the places that need code review

the goal 1 is solved by... "Adding {$T+} to the top of the unit"

the goal #2 exactly **requires**  making the legacy code ALL compilable in $T- (unless there is a bug in compiler like in Delphi xe2)

Arioch

  • Sr. Member
  • ****
  • Posts: 421
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #4 on: May 19, 2023, 08:18:57 pm »
Also, i admit i did a foolish thing by tinkering with "overrides" (but that is how we learn).
The options was there, just a page above (but also hard to find).

But here it seems to be another can of 22

The #3 screenshot is the project options for a package.

Sadly Laz IDE is designed in the asusmptiuon there is always exactly one EXE-project open, never less than one and never more than one.  :-/

So the package - part of the IDE de facto - inherits some option from EXE (namely this -Sy override from #2 screensshot) - but does NOT show it and does NOT give user way to override it.

Actually, in an ideal world i should not run into all this, as all i was doing was back-porting the Juha's patch.
The package would NOT inherit the -Sy switch in such a world, would had recompiled, and i would never learn all the things spelled.

This hiding if the essential page for packages... there probably is the reason, but still it was very cvonfusing catch 22.

Also, i told in the trackeer about synchronizing options in two pages.

I still wish the -Sy box in the #1 screenshot would get automatically selected just because the -Sy was manually added to the Overrides.

And i think (can not test if maybe already is) the -Sy checkboix should be synchronized with $T+ checkbox Juha just added.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 11338
  • Debugger - SynEdit - and more
    • wiki
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #5 on: May 19, 2023, 08:43:06 pm »
Also, i admit i did a foolish thing by tinkering with "overrides"
"Additions and overrides", I assume?

Quote
Sadly Laz IDE is designed in the asusmptiuon there is always exactly one EXE-project open, never less than one and never more than one.  :-/
Indeed, but even if multiple projects could be open at the same time: When you compile a project (or several in parallel) for each project there is exactly one set of settings.
(If you could compile several projects in parallel, you would need to change output path for ppu to avoid conflicts)

If you compile a project, then the settings of that project will be taken. "Additions and overrides" are meant to force options on selected (or all) packages.

Quote
So the package - part of the IDE de facto - inherits some option from EXE (namely this -Sy override from #2 screensshot)
Yes, but only if you explicitly specify that.

- If you don't use "Additions and overrides" (but maybe "custom options") => the package wont be affected.
- If you don't set a "*" as target of "Additions and overrides" then you can name packages you want to be affected (e.g. maybe your own package, with your own common code)

Quote
- but does NOT show it and does NOT give user way to override it.
Press the button "Show Options" on the bottom of the package-option dialog.

Indeed, you can't tell a package to prevent "Additions and overrides", if "Additions and overrides" target that particular package. That is the entire idea of "Additions and overrides" => override whatever the package really wants.

If you override package options, then you must take the same care that your options are "correct for the package" as you must do, when you edit the packages own options directly.

Btw, many package include $(IDEBuildOptions) => so if you change "Configure build Lazarus", then they all are affected. But the "Additions and overrides" have higher priority.

Quote
The package would NOT inherit the -Sy switch in such a world, would had recompiled, and i would never learn all the things spelled.
Don't use "Additions and overrides" or only specify the packages you want to be changed.

Add project only options in the "custom options" page of the project options.

Quote
Also, i told in the trackeer about synchronizing options in two pages.

I still wish the -Sy box in the #1 screenshot would get automatically selected just because the -Sy was manually added to the Overrides.
That be problematic.
- Users wouldn't see the difference if it was set, or just checked because of the "custom option"
- What if the custom option was removed later? Then the checkbox has to be undone too. Except maybe the user did switch the checkbox off and on again in between?

And you surely don't think that every option set by checkbox, should get an entry in "custom options"? (you can see it with the button  "Show Options" )

Best that could be done, is that unchecked boxes, that have the corresponding function set by other means, would be "marked" in some way. (a colored hint in the caption?)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 11338
  • Debugger - SynEdit - and more
    • wiki
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #6 on: May 19, 2023, 08:46:14 pm »
Also, i admit i did a foolish thing by tinkering with "overrides" (but that is how we learn).

Press F1 => it gets you here: https://wiki.freepascal.org/IDE_Window:_Compiler_Options#Additions_and_Overrides

Arioch

  • Sr. Member
  • ****
  • Posts: 421
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #7 on: May 19, 2023, 08:49:00 pm »
@Martin - that wiki page has nothing about "typed pointer" - it would not help me to find ways making $T+ default state

@Juha - i finally managed to put your patch into Laz 2.2. Like expected - it works. Thank a lot.

Sadly it is not synchronized with the -Sy window i just found above :-)

Have nice weekend, everyone. AFK

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 11338
  • Debugger - SynEdit - and more
    • wiki
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #8 on: May 19, 2023, 08:51:56 pm »
I still wish the -Sy box in the #1 screenshot would get automatically selected just because the -Sy was manually added to the Overrides.

On 2nd read: I didn't realize you overlayed the pop up window....
=> Ignore my previous reply on this



It is synchronized => with the memo on the "custom options" page.

Add "-Sy" in that memo, and it appears in that popup.




Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 11338
  • Debugger - SynEdit - and more
    • wiki
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #9 on: May 19, 2023, 08:55:48 pm »
@Martin - that wiki page has nothing about "typed pointer" - it would not help me to find ways making $T+ default state

Of course not => that is documented in the fpc docs. The Lazarus doc does not copy the entire fpc doc.

But that doc tells you, that "Additions and overrides" affects other package (targets). Which you seemed to be "somewhat discontent" with? (Or did I misread your statement)?

Sadly Laz IDE is designed in the asusmptiuon there is always exactly one EXE-project open, never less than one and never more than one.  :-/

So the package - part of the IDE de facto - inherits some option from EXE (namely this -Sy override from #2 screensshot) - but does NOT show it and does NOT give user way to override it.

Why would you want a way to override (in the package) the override (that you explicitly applied to that package)?

440bx

  • Hero Member
  • *****
  • Posts: 5457
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #10 on: May 19, 2023, 09:03:42 pm »
It feels almost like a bug in a strongly types language. -Sy or {$T+} brings type checking there. The above snippet fails to compile with -Sy, as it should.
I completely agree.  I would go even further and say that the fact that it compiles with {$T-} should be considered a bug.

Code: Pascal  [Select][+][-]
  1. -    Polygon(DC, @Points, 4, False);
  2. +    Polygon(DC, @Points[Low(Points)], 4, False);
"Points" is an array. Polygon() needs an address for its first element. "@Points" gives address for the first element but -Sy demands it explicitly with "@Points[Low(Points)]". It repeats the word "Points" and does not look as nice.
Is there a way to improve this?
Probably but, the way to "improve" it may itself be questionable.  One way would be to declare the Points as an untyped var, a fix I would consider extremely deficient and would be against.  Another way would be to declare points (in Polygon) as simply a pointer (that way any pointer type would satisfy the requirement - which is bad.)

Honestly, I think the current requirement is as it should be, that is "@Points[low(Points)]", that's precise, accurate and correct.  If some programmers are bothered by precision, accuracy and correctness then they shouldn't be using Pascal (or any Wirthian language.)

I am also of the opinion that if code produces a different result when compiled with $T+ instead of $T- then that code is inherently _incorrect_.  The example provided by @Martin_fr can be made to produce the same result independent of the setting of the $T directive.  IOW, I personally consider that code to be incorrect and, the mistake to be rather nasty because it is not obvious.

IMO, all units should compile with $T+ as well as $T- and produce the same result.  That would raise the confidence level that the code is correct and solid.

« Last Edit: May 19, 2023, 09:05:15 pm by 440bx »
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v4.0rc3) on Windows 7 SP1 64bit.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4571
  • I like bugs.
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #11 on: May 19, 2023, 10:58:04 pm »
I guess this also affects pointer arithmetic.

E.g.
Code: Pascal  [Select][+][-]
  1. for i := 0 to sizeof(MyQWord)-1 do
  2.   MySum := MySum + PByte( @MyQWord+i )^;

Will without typed pointer, add all the bytes in the QWord.
But with typed pointer it will  add 8 times i bytes to the address (inc to the next qword). And then take the first byte of each of those qwords.
Both will compile. Just the result will be different.
Oh, that is bad. It means one has to be careful when applying -Sy to existing code. Pointer arithmetic is nasty and prone to errors in any case.  Fortunately most Pascal code don't have it, unlike C code.

Why not add {$T-} to those units that depend on not being compiled with typed pointers?
(As long as current implementation, wiht typed pointers off, of course is correct.)
Yes, in LazUtils the Masks unit is the only one that needs {$T-}. It mixes pointers of Byte, Char and Integer and is optimized well.
FreeType has lots of pointers and-Sy gives many errors. I did not check other packages.

[Edit] Oops, that was not correct. There are many more units that fail with -Sy. Apparently adding a new option to the Configure "Build Lazarus" window was not enough. A clean build is required.
I am in the process of supporting -Sy in all Lazarus code, either by typecasts or by adding {$T-}.
« Last Edit: May 20, 2023, 01:22:30 pm by JuhaManninen »
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4571
  • I like bugs.
Re: Type checking of typed pointers, option -Sy, directive {$T+}
« Reply #12 on: May 19, 2023, 11:20:26 pm »
@Juha - i finally managed to put your patch into Laz 2.2. Like expected - it works. Thank a lot.
Sadly it is not synchronized with the -Sy window i just found above :-)
Yes, you can select any FPC option for the Custom options page using the All options dialog. It is not synchronized with options offered in the other pages. However it is not a problem. If you accidentally select the same option twice, it won't do any harm.

"All options" could be synchronized by making the Checkbox for an already selected option checked but disabled, and with a hint "Already selected elsewhere" or similar.
Patches are welcome. I don't see it as a high priority myself.
« Last Edit: May 20, 2023, 12:01:00 am by JuhaManninen »
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

 

TinyPortal © 2005-2018