All Projects FPC fpcprojects fpprofilerfpGUI Lazarus Packages PatchesLazarus CCR Mantis pas2js

View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0029946FPCRTLpublic2016-04-02 13:102017-03-17 08:49
ReporterPeterX Assigned ToMarco van de Voort  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionwon't fix 
Product Version3.0.0 
Summary0029946: GetTextExtentPoint32W() declaration
DescriptionFourth parameter of GetTextExtentPoint32W()
in REDEF.INC is OUT, not VAR.


Please see from MSDN:

BOOL GetTextExtentPoint32(
  _In_ HDC hdc,
  _In_ LPCTSTR lpString,
  _In_ int c,
  _Out_ LPSIZE lpSize
);
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Relationships

has duplicate 0032369 resolvedOndrej Pokorny Lazarus Declare out parameters as out, no var 
related to 0031527 new Lazarus var/out winapi parameters: GetCursorPos, GetCaretPos, GetWindowRect 

Activities

Marco van de Voort

2016-04-07 11:44

manager   ~0091815

redef.inc is for Delphi (or more Pascal style) compatibility, and Delphi defines it VAR.

Thaddy de Koning

2016-04-08 07:18

reporter   ~0091835

Last edited: 2016-04-08 07:23

View 3 revisions

 Same reason as discussed on the forum: the declaration as var predates the introduction of out in the language and was correct at the time.
By now it is less correct.
Note that there is a lot of work involved if all var's in de winapi that are really out should be "fixed". Doesn't warrant the effort,imo.
If Delphi doesn't fix it, why should we?

PeterX

2016-04-09 13:35

reporter   ~0091878

It would not break any code as all OUT declarations
simply ignore given VAR input values.
Which is correct by design, as MSDN declares the FN.

I don't like all these unneccessary hints.
For me it's enough to see relevant hints, warnings etc ...

I would do the work and fix it. If I had access.

Cyrax

2016-04-09 17:04

reporter   ~0091879

Using out can be risky because caller can finalize parameter, causing hard to spot runtime errors to occur.

See this link for more info : http://article.gmane.org/gmane.comp.ide.lazarus.general/86942

Thaddy de Koning

2016-04-09 17:06

reporter   ~0091880

Last edited: 2016-04-09 17:14

View 4 revisions

 You do not need access or a separate branch. Simply create patches, e.g. under windows I use tortoise svn, under linux simply the svn commandline commands.
Patches like that will usually be accepted very quickly if there is an urgency for it. If devels decide removing the hints is enough urgency the patches will be applied when properly written. The svn documentation will help you on how to do a patch and there is also some information in the wiki.
You can submit the patches right here.

As an example I attached the patch. see what happens ;)

Note the patch is only correct and tested for the declaration of GetTextExtentPoint32. I already saw much more candidates.

Thaddy de Koning

2016-04-09 17:11

reporter  

redef.inc.patch (1,859 bytes)    
Index: redef.inc
===================================================================
--- redef.inc	(revision 33446)
+++ redef.inc	(working copy)
@@ -603,9 +603,9 @@
 //function GetTextExtentExPointI(DC: HDC; p2: PWORD; p3, p4: Integer; p5, p6: PINT; var p7: TSize): BOOL;external 'gdi32' name 'GetTextExtentExPointI';
 function GetTextExtentExPointW(DC: HDC; p2: LPWSTR; p3, p4: Integer; p5, p6: PInteger; var p7: TSize): BOOL; external 'gdi32' name 'GetTextExtentExPointW';
 function GetTextExtentPoint(DC: HDC; Str: PChar; Count: Integer; var Size: TSize): BOOL;external 'gdi32' name 'GetTextExtentPointA';
-function GetTextExtentPoint32(DC: HDC; Str: PChar; Count: Integer; var Size: TSize): BOOL;external 'gdi32' name 'GetTextExtentPoint32A';
-function GetTextExtentPoint32A(DC: HDC; Str: LPCSTR; Count: Integer; var Size: TSize): BOOL; external 'gdi32' name 'GetTextExtentPoint32A';
-function GetTextExtentPoint32W(DC: HDC; Str: LPWSTR; Count: Integer; var Size: TSize): BOOL; external 'gdi32' name 'GetTextExtentPoint32W';
+function GetTextExtentPoint32(DC: HDC; Str: PChar; Count: Integer; out Size: TSize): BOOL;external 'gdi32' name 'GetTextExtentPoint32A';
+function GetTextExtentPoint32A(DC: HDC; Str: LPCSTR; Count: Integer; out Size: TSize): BOOL; external 'gdi32' name 'GetTextExtentPoint32A';
+function GetTextExtentPoint32W(DC: HDC; Str: LPWSTR; Count: Integer; out Size: TSize): BOOL; external 'gdi32' name 'GetTextExtentPoint32W';
 function GetTextExtentPointA(DC: HDC; Str: LPCSTR; Count: Integer; var Size: TSize): BOOL; external 'gdi32' name 'GetTextExtentPointA';
 //function GetTextExtentPointI(DC: HDC; p2: PWORD; p3: Integer; var p4: TSize): BOOL;external 'gdi32' name 'GetTextExtentPointI';
 function GetTextExtentPointW(DC: HDC; Str: LPWSTR; Count: Integer; var Size: TSize): BOOL; external 'gdi32' name 'GetTextExtentPointW';
redef.inc.patch (1,859 bytes)   

PeterX

2016-04-10 00:18

reporter   ~0091889

Thanks Thaddy.

I thought the BugTracker was the right place to report something.
With SVN, how can I comment my opinion about such an issue ?

Thaddy de Koning

2016-04-10 11:44

reporter   ~0091897

Last edited: 2016-04-10 11:45

View 2 revisions

 Well, you write your bug reports here on the bugtracker, use svn to create a patch and submit the patch after that here also (you can upload the patch file above).

Regarding the subject, please read this https://sergworks.wordpress.com/2016/04/07/on-the-out-parameter-specifier-in-delphi/ and the other link in that blog post on why it is not necessary to apply our patch.
In that light, I am considering to withdraw it.

PeterX

2016-04-17 22:04

reporter   ~0092065

>> In that light, I am considering to withdraw it.

So, if this is the right choice then Lazarus/FreePascal must suppress
the fully unneccessary hints caused by all these procedures and functions.

Whatfor are hints if they do say nothing or the opposite of what's meant to be !

Sven Barth

2016-04-22 13:56

manager   ~0092137

That's why they are *hints* and not *warnings*.

Regards,
Sven

Marco van de Voort

2016-05-01 16:52

manager   ~0092325

I'm not going to commit this. While there might be some minor benefits with respect to warnings and hints, I don't want to be inundated in requests to change all kinds of vars to out in the headers.

Sooner or later there will be mistake made, and IMHO it is simply not worth it.

If you don't want to see warnings and hints, disable them.

Thaddy de Koning

2016-05-02 11:52

reporter   ~0092332

Last edited: 2016-05-02 11:53

View 2 revisions

 I almost totally agree, with one or two/three remarks:
- New windows api's should be translated out when they are defined as out on msdn.
- The documentation isn't completely clear about the issue with interfaces (just COM. not CORBA) and reference counting should be documented as one such case of finalization.
- If and when Delphi changes their code, there is a real effort to be made. I don't think that will happen, however.

My suggestion is to put this on hold for a few years....

Marco van de Voort

2016-05-05 14:37

manager   ~0092412

Close. Not going to do this now.

Issue History

Date Modified Username Field Change
2016-04-02 13:10 PeterX New Issue
2016-04-07 11:44 Marco van de Voort Note Added: 0091815
2016-04-08 07:18 Thaddy de Koning Note Added: 0091835
2016-04-08 07:19 Thaddy de Koning Note Edited: 0091835 View Revisions
2016-04-08 07:23 Thaddy de Koning Note Edited: 0091835 View Revisions
2016-04-09 13:35 PeterX Note Added: 0091878
2016-04-09 17:04 Cyrax Note Added: 0091879
2016-04-09 17:06 Thaddy de Koning Note Added: 0091880
2016-04-09 17:11 Thaddy de Koning Note Edited: 0091880 View Revisions
2016-04-09 17:11 Thaddy de Koning File Added: redef.inc.patch
2016-04-09 17:13 Thaddy de Koning Note Edited: 0091880 View Revisions
2016-04-09 17:14 Thaddy de Koning Note Edited: 0091880 View Revisions
2016-04-10 00:18 PeterX Note Added: 0091889
2016-04-10 11:44 Thaddy de Koning Note Added: 0091897
2016-04-10 11:45 Thaddy de Koning Note Edited: 0091897 View Revisions
2016-04-17 22:04 PeterX Note Added: 0092065
2016-04-22 13:56 Sven Barth Note Added: 0092137
2016-05-01 16:52 Marco van de Voort Note Added: 0092325
2016-05-02 11:52 Thaddy de Koning Note Added: 0092332
2016-05-02 11:53 Thaddy de Koning Note Edited: 0092332 View Revisions
2016-05-05 14:37 Marco van de Voort Note Added: 0092412
2016-05-05 14:37 Marco van de Voort Status new => resolved
2016-05-05 14:37 Marco van de Voort Resolution open => won't fix
2016-05-05 14:37 Marco van de Voort Assigned To => Marco van de Voort
2017-03-17 08:49 Juha Manninen Relationship added related to 0031527
2017-09-03 22:10 Ondrej Pokorny Relationship added has duplicate 0032369