Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-pascal / 2008-08

Compiler option to check return value ignorance

16 messages · 5 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

leledumbo

Sometimes we forgot whether a function argument is passed by reference or by value. What if the result is actually important? For instance, consider the following (WARNING: True story):

function Align(Addr: Pointer; Alignment: PtrUInt): Pointer;

then we forgot that Addr is passed by value, so instead of stating

APointer:=Align(APointer,TheAlignment);

we only call

Align(APointer,TheAlignment);

which is harmful as we may need to have the pointer aligned. It's useful, believe me (Nimrod even doesn't allow it without a special statement).

PS: I think a warning (or note or hint, whichever suits best) is enough.

View this message in context: http://www.nabble.com/Compiler-option-to-check-return-value-ignorance-tp18958911p18958911.html Sent from the Free Pascal - General mailing list .
Joost van der Sluis
to leledumbo

Op woensdag 13-08-2008 om 01:27 uur [tijdzone -0700], schreef leledumbo:

Show quoted text 6 lines
Sometimes we forgot whether a function argument is passed by reference or by value. What if the result is actually important? For instance, consider the following (WARNING: True story):

That's not so strange story. I think this is one of the (if not the) most common mistakes of beginning programmers. And -as always- sometimes the most experienced also make this mistake.

And there's no way the compiler can detect this. Because this could be intentional. If you'll give a warning on such a construct, I think about 50% of the procedures would gresult in such a warning...

And the rules are simple: (for the beginners)

procedure Align(Addr: Pointer; Alignment: PtrUInt);
-> This procedure can't change any of it's parameters

procedure Align(const Addr: Pointer; Alignment: PtrUInt); -> This procedure can't change any of it's parameters, and Addr can't be changed inside the procedure also.

procedure Align(var Addr: Pointer; Alignment: PtrUInt);
-> This procedure can change Addr

If you really need those warnings, you could learn yourself to always use 'const', 'var' or 'out'. And never without any of these words.

Joost

leledumbo
to Joost van der Sluis
Joost van der Sluis wrote
And there's no way the compiler can detect this.

I don't think so. The compiler understand that a procedure doesn't return a value while a function does (i.e. trying to return a value from a procedure causes compile error). I haven't checked how compiler formats each function/procedure in the PPU but I think there should be an information whether it's a function or procedure. So, I think it's possible to detect if a function call is assigned to a variable or not.

View this message in context: http://www.nabble.com/Compiler-option-to-check-return-value-ignorance-tp18958911p18960053.html Sent from the Free Pascal - General mailing list .
Vinzent Höfler
to leledumbo
leledumbo wrote 10 lines
Joost van der Sluis wrote: > And there's no way the compiler can detect this. > I don't think so. The compiler understand that a procedure doesn't return a value while a function does (i.e. trying to return a value from a procedure causes compile error). I haven't checked how compiler formats each function/procedure in the PPU but I think there should be an information whether it's a function or procedure. So, I think it's possible to detect if a function call is assigned to a variable or not.

Yes. Turn Extended Syntax off {$X-}, then a function result must be assigned to a variable. At least it used to be that way.

I was used to it, and in the case I really needed to ignore the result (and rather just wanted the side-effects of the function) I wrote things like:

|if (IOResult  0) then {null};

Well, then ... I started with classes:

Now, constructors seem to actually return something (not surprisingly), so "inherited Create" did not compile anymore (and I remember that any attempt to assign this "result" to a dummy variable even failed). So, at that time I turned the extended syntax stuff back on.

So, in theory your problem is already solved in the compiler (even TP6 already had it), but alas, in practice it's not really usable.

Vinzent.

leledumbo
to Vinzent Höfler
Vinzent Höfler wrote 8 lines
Yes. Turn Extended Syntax off {$X-}, then a function result must be assigned to a variable. At least it used to be that way. ... so "inherited Create" did not compile anymore (and I remember that any attempt to assign this "result" to a dummy variable even failed). So, at that time I turned the extended syntax stuff back on.

So, if I don't use any classes, turning Extended Syntax Off would be OK? What's activated by this Extended Syntax anyway? Any document/article about it?

View this message in context: http://www.nabble.com/Compiler-option-to-check-return-value-ignorance-tp18958911p18960807.html Sent from the Free Pascal - General mailing list .
Vinzent Höfler
to leledumbo
leledumbo wrote 11 lines
Vinzent Höfler wrote: > Yes. Turn Extended Syntax off {$X-}, then a function result must be > assigned to a variable. At least it used to be that way. > ... > so "inherited Create" did not compile anymore (and I remember that any > attempt to assign this "result" to a dummy variable even failed). So, at > that time I turned the extended syntax stuff back on. > So, if I don't use any classes, turning Extended Syntax Off would be OK?

I guess so. Haven't tried it in years, though.

What's activated by this Extended Syntax anyway? Any document/article about it?

http://www.freepascal.org/docs-html/prog/progsu108.html#x116-1160001.2.32

With Turbo/Borland Pascal there were some other subtle differences (like
- I think - treating an array of char as string/PChar), but the main effect was the throwing away of function results.

Vinzent.

Joost van der Sluis
to leledumbo: I don't think so. The compiler understand that a procedure doesn't return a value while a…

Op woensdag 13-08-2008 om 02:52 uur [tijdzone -0700], schreef leledumbo:

Show quoted text 11 lines
Joost van der Sluis wrote: > > And there's no way the compiler can detect this. > I don't think so. The compiler understand that a procedure doesn't return a value while a function does (i.e. trying to return a value from a procedure causes compile error). I haven't checked how compiler formats each function/procedure in the PPU but I think there should be an information whether it's a function or procedure. So, I think it's possible to detect if a function call is assigned to a variable or not.

Ok, so I misunderstood you. You want that the compiler complains if you don't assign the result of a function. While that can be done, I don't think you want that. This functionality is used far too often, and those warnings would become very annoying.

Joost.

Vinzent Höfler
to Joost van der Sluis
Joost van der Sluis wrote
Ok, so I misunderstood you. You want that the compiler complains if you don't assign the result of a function. While that can be done, I don't think you want that.

Well, don't decide for others, please. I am a stupid programmmer, I am making mistakes all the time, and any feature that could possibly stop me from making such is - in my opinion - a useful feature.

I *wanted* it, even have used that feature for years - until "inherited Create" came disguised as a function. Unfortunately {$X} can't be used locally and always accounts for the whole compilation unit, so you can't even use it to temporarily switch on the extended syntax in the few places where it would be needed (like I sometimes turn off range checks locally) and any other attempts to circumvent that problem just failed.

I just tested again and found something weird. Turning off "Extended Syntax" actually disables compilation of constructors at all, so it wasn't even only a problem with the "inherited Create" as I remembered.

Look at this:

-- 8< -- snip --
{$MODE OBJFPC}
{$X-}
program
    Ext_Syntax;
type
    Foo = class (tObject)
    public
       constructor Create;
    end;
constructor Foo.Create;
begin
    inherited Create;
    if Assigned (inherited Create) then {null};
end;
var
    A : tObject;
begin
    A := Foo.Create;
end.
-- 8< -- snip --

Compiles fine with {$X+} (line 2). Now let's turn extended syntax off as shown and wonder:

|ext_syntax.pas(16,4) Error: Illegal expression

Ok, line 16 is the one with the "inherited Create" on its own. Meets the expectation, although it might not be intuitive, but well, even constructors can fail, so it might not even be the worst idea to check that. ;)

Workaround in the line 17 (dummy test) seems to compile though, so let's disable line 16 completely:

|ext_syntax.pas(15,1) Error: Operation ">" not supported for types
|"Pointer" and "Pointer"

Huh? Line 15? The "begin" line?

This happened a couple of years ago with 2.0.0, still happens with 2.0.2. Haven't given it a shot in 2.2.x yet, but at least now I remember why I had to turn on extended syntax for the whole project back then.

This functionality is used far too often, and those warnings would become very annoying.

It might not be common sense anymore, but the term "function" is often overloaded as a "subroutine without side-effects". Means, if you don't interprete the return value you may as well not call course, in practice, far too many "functions" have side-effects and often it's just the side-effect you need. But that doesn't mean that {$X-} wouldn't be .

Vincent Snijders
to Vinzent Höfler
Vinzent Höfler wrote 10 lines
Joost van der Sluis wrote: > Ok, so I misunderstood you. You want that the compiler complains if you > don't assign the result of a function. While that can be done, I don't > think you want that. Well, don't decide for others, please. I am a stupid programmmer, I am making mistakes all the time, and any feature that could possibly stop me from making such is - in my opinion - a useful feature.

Yes, but this feature doesn't because it gives so many false positives that I end up ignoring such warnings completely.

Vincent

Vinzent Höfler
to Vincent Snijders
Vincent Snijders wrote 13 lines
Vinzent Höfler schreef: > Joost van der Sluis wrote: > >> Ok, so I misunderstood you. You want that the compiler complains if you >> don't assign the result of a function. While that can be done, I don't >> think you want that. > > Well, don't decide for others, please. I am a stupid programmmer, I am > making mistakes all the time, and any feature that could possibly stop > me from making such is - in my opinion - a useful feature. Yes, but this feature doesn't because it gives so many false positives that I end up ignoring such warnings completely.

IBTD. I actually *used* that feature, until it became unusable due to the inability to compile don't get me wrong, but
maybe I am in a better position to judge its usefulness. Especially because it doesn't generate warnings, it simply refuses to compile. So you can't even choose to ignore it.

I just grep'ped the actual sources of the project (about 70K LOC) and I found exactly four occurences of my "if Ignore_Result() then {null};" pattern with FPC runtime routines. There are others, but those routines access hardware and on some occasions I really need to ignore failures there.

2x IOResult
1x SysUtils.DeleteFile
1x BaseUnix.fpChMod

Twice more of this pattern occured to avoid the compiler hint of unused parameters. So if "false positives" is an indicator for uselessness of a compiler message, I'd tend to doubt the usefulness of that.

Well, I even think that pointing the maintenance programmer to the fact that I deliberately chose to ignore the return value instead of possibly having forgotten it, might help understanding the code better.

At last, there's a pretty good reason why in some (C-)coding standards it is even mandatory to cast unused function results to (void). Although I'd sure find it a bit annoying to write "(void) printf ("Foo");". But contrary to Pascal procedures, C only knows functions and almost any of it returns something that's non-void, useful or not. ;)

Vinzent.

Vincent Snijders
to Vinzent Höfler
Vinzent Höfler wrote 18 lines
Vincent Snijders wrote: > Vinzent Höfler schreef: >> Joost van der Sluis wrote: >> >>> Ok, so I misunderstood you. You want that the compiler complains if you >>> don't assign the result of a function. While that can be done, I don't >>> think you want that. >> >> Well, don't decide for others, please. I am a stupid programmmer, I >> am making mistakes all the time, and any feature that could possibly >> stop me from making such is - in my opinion - a useful feature. > > Yes, but this feature doesn't because it gives so many false positives > that I end up ignoring such warnings completely. IBTD. I actually *used* that feature, until it became unusable due to the inability to compile don't get me wrong, but maybe I am in a better position to judge its usefulness.

Sorry, that I misjudge your position to judge its usefulness and I misunderstood the difference between a new warning and an old fatal error.

Vincent

Jeff Wormsley
to Vinzent Höfler: IBTD. I actually *used* that feature, until it became unusable due to the inability to…
Vinzent Höfler wrote 14 lines
I just grep'ped the actual sources of the project (about 70K LOC) and I found exactly four occurences of my "if Ignore_Result() then {null};" pattern with FPC runtime routines. There are others, but those routines access hardware and on some occasions I really need to ignore failures there. 2x IOResult 1x SysUtils.DeleteFile 1x BaseUnix.fpChMod Twice more of this pattern occured to avoid the compiler hint of unused parameters. So if "false positives" is an indicator for uselessness of a compiler message, I'd tend to doubt the usefulness of that.

So, if I read you correctly, if you call a function that you don't care what the result is, instead of:

Begin
  SomeFunction(123); // Ignore result
End;

... you would have us write:

Var Dummy: SomeType;
Begin
  Dummy := SomeFunction(123);
End;

... or else get warnings from the compiler?

I'm afraid there are tons of code out there that this would effect. I don't think any of the maintainers of such code would appreciate that change much. Maybe, just maybe, an optional {$MODE ULTRA_STRICT_FUNCTION_RESULT_USE} type compiler option, whose default was off, would be ok, but otherwise such a change would be far worse than the "problem" you are trying to address (someone improperly not using the function result when they were supposed to).

Jeff.

I haven't smoked for 1 year, 11 months and 3 weeks, saving $3,274.62 and not smoking 21,830.81 cigarettes.
Vinzent Höfler
to Jeff Wormsley

-------- Original-Nachricht --------

Show quoted text 36 lines
Datum: Wed, 13 Aug 2008 16:02:10 -0400 Von: Jeff Wormsley An: FPC-Pascal users discussions Betreff: Re: [fpc-pascal] Compiler option to check return value ignorance Vinzent Höfler wrote: > I just grep'ped the actual sources of the project (about 70K LOC) and > I found exactly four occurences of my "if Ignore_Result() then > {null};" pattern with FPC runtime routines. There are others, but > those routines access hardware and on some occasions I really need to > ignore failures there. > > 2x IOResult > 1x SysUtils.DeleteFile > 1x BaseUnix.fpChMod > > Twice more of this pattern occured to avoid the compiler hint of > unused parameters. So if "false positives" is an indicator for > uselessness of a compiler message, I'd tend to doubt the usefulness of > that. So, if I read you correctly, if you call a function that you don't care what the result is, instead of: Begin SomeFunction(123); // Ignore result End; ... you would have us write: Var Dummy: SomeType; Begin Dummy := SomeFunction(123); End; ... or else get warnings from the compiler?

Apart from the fact that I write

|if not SomeFunction  Constant_Of_Sometype then {null};

yes. And it's a compile time error.

I'm afraid there are tons of code out there that this would effect. I don't think any of the maintainers of such code would appreciate that change much.

I didn't ask for a change. The feature is already there.

All I would ask for that {$EXTENDED_SYNTAX OFF} wouldn't be equivalent to "I refuse to compile constructors" (and maybe in turn allow the use of "inherited Create" without this strict check).

Sure, I won't force anyone to use it. But I know environments where your employer would force you to. Even worse, there are environments where merely assigning the result to a dummy variable wouldn't be enough to satisfy the strict checks, you'd have to do something useful with it. And to be honest, there aren't many cases where you really can ignore the result of the function and keep the good conscience of having it done right. If a function in Pascal returns something it's usually useful.

Vinzent.

GMX startet ShortView.de. Hier findest Du Leute mit Deinen Interessen! Jetzt dabei sein:
Vinzent Höfler
to Vinzent Höfler
|if not SomeFunction Constant_Of_Sometype then {null};

Of course, usually it's without the "not"... ;) And with Boolean functions it's a bit easier.

And I should have added that I never put in comments what I can easily put in code. At first, comments don't get compiled, and second, nobody never seems to read them, anyway.

Vinzent.

GMX startet ShortView.de. Hier findest Du Leute mit Deinen Interessen! Jetzt dabei sein:
Jeff Wormsley
to Vinzent Höfler: -------- Original-Nachricht -------- Apart from the fact that I write |if not…
Vinzent Höfler wrote
And to be honest, there aren't many cases where you really can ignore the result of the function and keep the good conscience of having it done right. If a function in Pascal returns something it's usually useful.

There are many cases like that, at least in my experience. Of course, I write business apps, not medical systems or rocket guidance, so my tolerance for not dotting every i and crossing every t is probably higher than others, but at the same time, I certainly wouldn't want to be forced to check for conditions that aren't likely to ever occur. The extra expense in development time isn't worth the risk of not doing the checks. That cost vs risk analysis will be different for everyone.

Jeff.

I haven't smoked for 1 year, 11 months and 4 weeks, saving $3,277.77 and not smoking 21,851.82 cigarettes.
Vinzent Höfler
to Jeff Wormsley

Datum: Thu, 14 Aug 2008 08:48:58 -0400
Von: Jeff Wormsley

be forced to check for conditions that aren't likely to ever occur. The extra expense in development time isn't worth the risk of not doing the checks. That cost vs risk analysis will be different for everyone.

Consensus these days is that the earlier a bug is detected the cheaper it is to fix. So business-wise a potential bug that doesn't even slip through the compiler has zero cost for fixing it.

Another ten seconds of typing work is worth the effort to avoid potentially hours of searching for a nasty bug in software that's already deployed in the field. Especially when it comes to maintenance, where the poor guy has to decide if the ignorance was intentional or not each time.

Well, YMMV.

Vinzent.

Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer