View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0029946||FPC||RTL||public||2016-04-02 13:10||2017-03-17 08:49|
|Reporter||PeterX||Assigned To||Marco van de Voort|
|Summary||0029946: GetTextExtentPoint32W() declaration|
|Description||Fourth parameter of GetTextExtentPoint32W()|
in REDEF.INC is OUT, not VAR.
Please see from MSDN:
_In_ HDC hdc,
_In_ LPCTSTR lpString,
_In_ int c,
_Out_ LPSIZE lpSize
|Tags||No tags attached.|
|Fixed in Revision|
||redef.inc is for Delphi (or more Pascal style) compatibility, and Delphi defines it VAR.|
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?
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.
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
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.
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)
I thought the BugTracker was the right place to report something.
With SVN, how can I comment my opinion about such an issue ?
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.
>> 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 !
That's why they are *hints* and not *warnings*.
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.
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....
||Close. Not going to do this now.|
|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|