Launching a Lazarus application in a freshly installed Linux system, I found a condition which may be quite confusing for a normal user: the main form was shown, the program didn't work, and there was no way to close the form, which could only be closed by a killall from command line.

Launching from command line one could only see a
TApplication.HandleException Access Violation, which isn't much telling for a normal user either.

It turned out that the reason was simply that the default AppConfigDir (~/.config/ ) wasn't there, and therefore in the two usual lines

AppConfigFileName:= GetAppConfigFile(False);
ini := TIniFile.Create(AppConfigFileName);

the second line was generating the access violation.

Desktop specs tell where configuration data should go, but they don't guarantee that the directory exists. Other applications take care of creating if it doesn't exist, but its presence depends on which applications you launch.

Of course, once one knows, one can use ForceDirectories in the application code, but it would be much more user friendly if XdgConfigHome (or SysConfigDir when it will be implemented) in sysutils took care of that. You ask for the default configuration path to put your data in, and you get a sane and *existing* path. It would also be nice if TIniFile.Create didn't generate an Access Violation if the file can't be created.

Should I open an issue on the bugtracker on this subject, or I'm the only one to think that it's a bug/required feature?

Giuliano

If I were you, I'd open an issue. I agree this is a problem.

It is not a problem.

You should READ the documentation when using functions:

http://www.freepascal.org/docs-html/rtl/sysutils/getappconfigdir.html

Specifically

"This does not mean that the directory exists, or that the user can write in this directory (especially if Global=True). It just returns the name of the appropriate location."

So it is not a bug.

We can maybe have an appropriate 'AllowCreate' or 'EnsureDir' parameter or so. But for system config directories, this is nonsense, so it should be optional.

Michael.

It is indeed not a problem if this is user code, not default code in a Lazarus application. Didn't understand Giuliano's message to mean that.

Il 09/02/2013 10:25, Michael Van Canneyt ha scritto:

I can easily accept that it's not a bug, but you must agree with me that it's a problem.

If every user application must include code between GetAppConfigxx and TIniFile.Create to extract the path and ensure that it exists, even for the trivial case Global=false, then it's reasonable that the right place for that code is in system functions, and not in user application.

Maybe the right thing to fix isn't GetAppConfigxx, but TInifile.Create, which doesn't return any warning if the file path doesn't exist, and will just SigSev on a subsequent ini.WriteWhatever.

TIniFile silently creates the file if it doesn't exist, letting you think that it takes care of everything, then why shouldn't it also create the path if it doesn't exist? (provided it's writable, of course, but Global=true requires special handling in any case, it's not for everybody).

Giuliano

Yes.
Like I said, we can imagine a parameter 'AllowCreate' or 'ForceCreate'.

Michael.

Michael Van Canneyt wrote

Like I said, we can imagine a parameter 'AllowCreate' or 'ForceCreate'.

GetAppConfigDir(Global : Boolean: ForceCreate: Booelan = false) : String;

While this can easily be implemented without breaking existing code, this still does not solve the original problem.
If creating the directory fails, the function does not return an errorcode, unless we decide to let it raise an exception in this case..

Common programming sense would be to query the desired location, and if it does not exists, try and create it,(for this the ForeceCreate parameter is usefull) __and check__ if this went OK.

So, no matter how we change the GetAppConfigDir() function, it will always require code for checking if all went OK.

Bart

Which is why I said 'we can imagine':
I didn't investigate all angles yet...

There is only one way:
if the ForceCreate is implemented and if creation fails, an exception will be raised.

In which case the
TIniFile.Create(...)
and so forth will not be executed, and probably many other things will go wrong as well....

Michael.

And in this case the OP still needs to do an extra check (try..except), so basically this does not solve his "problem" because it is unsolvable without the programmer checking/validating what GetAppConfigDir did.

function GetAppConfigDir(Global: Boolean; ForceCreate: Boolean; WhatToDoIfCreationFails: TProcedure) would "solve" the original problem ;-)

Bart

Bart

Which is why I always write the code to check/create the dir myself.

But having the check/create in a central place can have its advantages.

Michael.

The advantages I can see that it would make easier using VendorName, and, most of all,
that once it's been written and debugged it's written and debugged for all users, while application code must be written and debugged each time!

Giuliano

That's why I think they should stay as two separate prodecures.

Regards,
- Graeme -

Looking more carefully into the matter, the real inconsistency is not in GetAppConfigFile, or in GetAppConfigDir, which just do what they're meant for, but in TIniFile:

1) When you use it to *read* configuration parameters, if the file doesn't exist it silently just passes you the default values. This is normal, because the first time the program runs it didn't yet have the chance to write configuration data.

2) When you use it to *write* configuration parameters, it silently creates the file, and puts there the parameters. This again is normal, because the first time the program run the configuration file isn't there yet.

3) But if also the *path* to the file doesn't yet exist, it just crashes without rising an exception that the user application can handle somehow.

This is rather unpleasant, because the path provided by GetAppConfigXx does usually exist, so you have an application which 99% of the times works just fine, and 1% of the times crashes without telling why.

This was perhaps tolerable when GetAppPathConfigXx did not exist, and it was developer responsibility to provide a reasonable path, taking into account different target system, but today I'd expect TIniFile to take care also of forcing the appropriate path, and to raise an exception if it fails.

Giuliano

[...] 3) But if also the *path* to the file doesn't yet exist, it just crashes without rising an exception that the user application can handle somehow.

I get an exception in this case:
  inifile:=TIniFile.Create('/does/not/exist/bla.ini');
  inifile.WriteString('Sec','First','1');
  inifile.UpdateFile;

This is rather unpleasant, because the path provided by GetAppConfigXx does usually exist, so you have an application which 99% of the times works just fine, and 1% of the times crashes without telling why.

Even if the directory exists the write may fail. So you must use a try..except anyway.

Mattias

Giuliano Colla wrote

3) But if also the *path* to the file doesn't yet exist, it just crashes without rising an exception that the user application can handle somehow.

This is incorrect. It does tell you why it fails.

This is rather unpleasant, because the path provided by GetAppConfigXx does usually exist, so you have an application which 99% of the times works just fine, and 1% of the times crashes without telling why.

It tells you *exactly* why it 'crashes':

home: >./ti
An unhandled exception occurred at $000000000042AE87: EFCreateError: Unable to create file "/tmp/path/to/nothing.txt" $000000000042AE87

home: >cat ti.pp
program ti;
uses inifiles;
begin
   with TMemIniFile.create('/tmp/path/to/nothing.txt') do
     try
       WriteString('A','B','C');
       UpdateFile;
     finally
       Free;
     end;
end.

However, I agree that it should try to create the path as well. I have adapted the implementation.

You'll get an error telling you if it failed to create the dir: An unhandled exception occurred at $0000000000427F7E: EInOutError: Could not create directory "/my/path/to/" $0000000000427F7E

Which means that you are still (as previously) responsible for catching possible errors. But these errors should be less frequent with this change.

Michael.

Il 11/02/2013 09:21, Michael Van Canneyt ha scritto:

This is true only if you invoke the program from command line, which is something the end user will never do. Consider that this sort of errors doesn't pop up when you're developing, but rather after you've deployed your application.
Let's assume that saving configuration is on a FormClose procedure, as usual, and you have a try..finally construct, as usual. Your user starts the program by clicking on a launching Icon. When he clicks on a Close button, if the path doesn't exist the program won't be terminated, and he's left with a window he can't close. On a terminal you can see

TApplication.HandleException Unable to create file "/home/colla/.pippo/uclc.cfg" Stack trace:
$080BAF29
[..]
exception at 080BAF29:
Unable to create file "/home/colla/.pippo/uclc.cfg".

But no error is shown on the screen (try..finally has trapped it), and the form is not closed. Then the user will attempt to click again on the close button of a form which is already "almost" closed. On a terminal you can see:

TApplication.HandleException Access violation
Stack trace:
[..]
$082A9B6D GTK2WSBUTTON_CLICKED, line 2417

But again nothing on the screen. The program hangs here. From command line you need a ^C to terminate. Otherwise you need a killall. Which isn't a very good situation for an end user.

Thank you, attempting to create the path is a welcome improvement. It makes TIniFile behavior much more consistent.

However, what I've learned from this episode is that while attempting to save configuration data using TINIFile on program termination you should never use a try..finally construct, as it's suggested everywhere, but rather a try..except (or both). Otherwise you'll never be able to catch possible errors.

Giuliano

That is not the problem of TIniFile, but of the application programmer.

TIniFile properly reports any error, as it should, with an exception. It is always up to the application programmer to deal with such cases.

That this is sometimes tricky in a GUI application, cannot be helped.

Michael.

I'd suggest that there are two separate cases here. In the first case, near the start of a program you decide whether there is a preexisting .ini file, create it if not, and if necessary update it to contain e.g. the default name of a backend server. In the second case, at the the end of a program run you save the current session's state.

Il 11/02/2013 12:40, Mark Morgan Lloyd ha scritto:

I was speaking of the second case(saving configuration data to your .ini file).
When attempting to load configuration data, a try..finally is ok. If no configuration data are available you already have a built-in mechanism to provide default values.
But when attempting to save the current session's state at the end of the program, a try..finally will mask any error, and possibly lead to an unpleasant lock up.

Giuliano

Il 11/02/2013 12:35, Michael Van Canneyt ha scritto:

It can be helped both by reducing the likelihood of the event, as you've done, and by not suggesting to use in that case a try..finally construct in a GUI application, which would mask the errors. This suggestion comes historically from Delphi literature, but fpc/Lazarus developers should never forget that they're smarter than Delphi people. ;-)

Giuliano

The "try..finally" itself will not mask the error. The error will propagate to the next surrounding "try..except" handler (executing the code in all finally sections it encounters) and normally this will result in a dialog being displayed (if you did not change Application.OnException). It could be however that you try to create the configuration file when the usage of GUI is no longer possible (I don't know).

Regards,
Sven

Giuliano Colla wrote

This is true only if you invoke the program from command line, which is something the end user will never do.

Wrong. This is something you are doing wrong in your GUI application then. I have loads of fpGUI 'gui' applications, that report exceptions just fine to the use - no matter how they launched that application.

Regards,
- Graeme -

I didn't suggest anything. I wrote 'deal with such cases'. How, this I leave up to the programmer.

But I will say this:
trying to format the user's harddrive or attempt to fry his monitor is usually the wrong approach ;)

Michael.

But you should have already ascertained early in the program- when the .ini was created (possibly from a template) or read- that the path etc. was accessible. What's more you're typically doing this before the main program logic is started, so you shouldn't get caught by finalization that's only there to clear up after the main program.

So this is, basically, bad application coding.

Besides which, while I agree that your original point

It turned out that the reason was simply that the default AppConfigDir (~/.config/ ) wasn't there, and therefore in the two usual lines

illustrates something that is inconvenient, my understanding is that different distreaux (not to mention the preference of different system owners) use ~/.config to a varying extent. If anything, it is the fault of AppConfigDir for indicating a directory without raising an exception pointing out that it doesn't yet exist :-)

I wasn't speaking of you. You didn't. But too many examples show it that way. Programmers are influenced by examples.

But I will say this: trying to format the user's harddrive or attempt to fry his monitor is usually the wrong approach ;)

Creating a .config/ directory is somehow more in the natural programming practice, than formatting user's hard drive, or frying his monitor. We'll leave those practice to Redmond developers....

Giuliano

I agree with you. I just followed blindly the myriad of examples which surround with a try..finally the statements after a TIniFile.Create, to ensure that TIniFile.Free is executed, forgetting that in a GUI application this is not the proper way to do it.
With a try..except any error in between would be properly handled, and Free may be called safely as required.

Giuliano

Lost in the shuffle, wasn't this originally found in Lazarus? Sounds like a bug report for incorrect handling of INI files when the config directory is missing needs to be filed in their bug tracker.

Jeff.

Mark Morgan Lloyd wrote

If anything, it is the fault of AppConfigDir for indicating a directory without raising an exception pointing out that it doesn't yet exist :-)

And AppConfigDir/File is documented as not guaranteeing that the path to the directory or file exists. This also includes upper parts of the path's tree.

Regards,
Sven

Just to prove my point: I wrote the following little test program in Lazarus. Add a button to a empty form and assign the OnCreate and OnDestroy events of the form and the OnClick event of the button and add the following code to them:

=== snippet begin ===

procedure TForm1.FormCreate(Sender: TObject);
begin
   try
     raise Exception.Create('FormCreate');
   finally
     MessageDlg('FormCreate Finally', mtInformation, [mbOK], 0);
   end;
end;
procedure TForm1.Button1Click(Sender: TObject);
begin
   try
     raise Exception.Create('Button1Click');
   finally
     MessageDlg('Button1Click Finally', mtInformation, [mbOK], 0);
   end;
end;
procedure TForm1.FormDestroy(Sender: TObject);
begin
   try
     raise Exception.Create('FormDestroy');
   finally
     MessageDlg('FormDestroy Finally', mtInformation, [mbOK], 0);
   end;
end;

=== snippet end ===

If you now run the application (for now I assume you run it without a debugger or outside the IDE) you first get the "FormCreate Finally" dialog, then a "Exception "FormCreate" occured" dialog and then the form will be display. An analogous event will happen if you click the button and as well when you close the form.

So it would be nice if you'd show where exactly no error message is displayed for you (preferrably with a compiling code example).

Regards,
Sven

And here is an example, what happens when a program blindly forces directories:

http://www.administrator.de/images/content/e06eaa5bdbdca8c9602d9d0f7c5b4e09-1.gif

It seems some system variables under Windows sometimes contain gibberish. But that's off topic.

Mattias

Hui O.o It shows nicely that NTFS and Windows support Unicode characters :P

It seems some system variables under Windows sometimes contain gibberish. But that's off topic.

Either that or the application did use the corresponding APIs correctly...

Regards,
Sven

DaWorm wrote

Lost in the shuffle, wasn't this originally found in Lazarus? Sounds like a bug report for incorrect handling of INI files when the config directory is missing needs to be filed in their bug tracker.

INI files and related stuff are part of the fcl library (packages/fcl-base), not of the Lazarus library.

Giuliano

On Mon, Feb 11, 2013 at 1:48 PM, Giuliano Colla <

Sorry, went back and look, I misread "a Lazarus application" as "the Lazarus application" and thought it was Lazarus itself saving its settings.

As Emily Littella used to say, Never Mind.

Jeff.

I extracted from my code a minimal example, and I too can see the exception dialog, which doesn't show in the main application.

I don't see why the extra code should create a problem, but, by adding one piece after the other I should be able to find out where the problem comes from. However, replacing the original try..finally with a try..except, the main application behaves properly, but in that case it's the application which pops up the error dialog.

Giuliano

It would be nice if you could minimize the problemematic code further step by step so that we can see what caused the "missing dialog". Maybe it's a bug somewhere else...

Regards,
Sven

Platform: Win32, FPC2.7.1, Virusscanner Avira Free Antivirus

The compiled program is assumed to be a virus.

Name:TR/Crypt.XPACK.Gen2 (according to Avira, shall be a trojan)

Compile line:
ppc386 -vv -al -CioOrt -Cs6000000 -gclt -Mobjfpc -O1 -OpPENTIUM

With the external linker ( -Xe ) the program is not a virus and works.

Shall I make a bug report from that? I think I can not produce a small test program, the problem occurs only with one program, others do compile without problem.

Gerhard

Yes, for avira (assuming your PC is not infected) since the program is apparently not a virus.

I think I can not produce a small test program, the problem occurs only with one program, others do compile without problem.

IMHO, should be reported to the AV manufacturer.

btw, if you strip debug info, is it still reported? Because Kaspersky had/has that problem. And several other AV seems to get signatures from the same source.

On report they updated their signatures. Apparently whitelisted. The reported exe, now is fine. A byte by byte identical exe, with the exception of one single digit, in a textual date string differing, is still reported.

@Martin:

Compile line: ppc386 -vv -al -CioOrt -Cs6000000 -Mobjfpc -O1 -OpPENTIUM (dropping the -gclt) helped.

It seems you were right.

Gerhard

I switched to AVAST free Antivirus now, the problem does not occur here.

This is not a pascal question, but maybe can give me a hint if AVAST is good or which software is more recommendable.
(I do not really trust the tests I find in the internet because I do not know
who pays them)

Gerhard

I admit that I was slightly trolling there, since Giuliano was complaining about exceptions that he wasn't seeing (because, it turns out, he wasn't catching them).

However I feel that my point stands: if the program opens, checks and closes the .ini file before the main logic starts, then it's possible to keep those initial activities separate from any try-finally required by the application logic. This initial activity obviously still requires both a try-finally and a try-except, which is where he was going wrong with his initial attempt.

I found myself recently quite comfortable using ZoneAlarm.

L.

Sven Barth wrote

It would be nice if you could minimize the problemematic code further step by step so that we can see what caused the "missing dialog". Maybe it's a bug somewhere else...

I've made some further experiments with my minimal test. Test form is just a Form with a Close button (BitBtn Kind=bkClose). The full code is the following:

unit uinitfile;
{$mode objfpc}{$H+}
interface
uses
   Classes, SysUtils, FileUtil, Forms, Controls, Graphics, Dialogs, Buttons,IniFiles;
type
   { TForm1 }
   TForm1 = class(TForm)
     BitBtn1: TBitBtn;
     procedure FormClose(Sender: TObject; var CloseAction: TCloseAction);
     procedure FormCreate(Sender: TObject);
   private
     { private declarations }
   public
     { public declarations }
   end;
var
   Form1: TForm1;
   ini: TIniFile;
   AppConfigFileName: string;
   SomeData: boolean;
   SomeOtherData: boolean;
implementation
{$R *.lfm}

{ TForm1 }

procedure TForm1.FormCreate(Sender: TObject);
begin
  AppConfigFileName:= '/not/existing/path/inifile.cfg';
  ini := TIniFile.Create(AppConfigFileName);
  try
    SomeData:= ini.ReadBool('Section','Val',true);
    SomeOtherData:= ini.ReadBool('Section','Val1',true);
  finally
    ini.Free;
  end;
end;
procedure TForm1.FormClose(Sender: TObject; var CloseAction: TCloseAction);
begin
   ini:= TIniFile.Create(AppConfigFileName);
{$DEFINE Finally}
{$IFDEF Finally}
   try
     ini.WriteBool('Section','Val',SomeData);
     ini.WriteBool('Section','Val1',SomeOtherData);
   finally
     ini.Free;
   end;
{$ELSE}
   try
     ini.WriteBool('Section','Val',SomeData);
     ini.WriteBool('Section','Val1',SomeOtherData);
   except
     MessageDlg('Error while writing '+AppConfigFileName,mtError,[mbOK],0);
   end;
   ini.Free;
{$ENDIF}
end;
end.

One may test two conditions: with try..finally and with try..except. Even with this minimal sheds some light on the matter.

Try..finally situation, with IDE and Debugger.
Press the Close button. A debugger error notification is shown. Pressing "Continue", an error dialog is displayed: "Unable to create ... press OK to Ignore...etc.":
Pressing OK (no harm should come, just the file can't be written) the form isn't closed as it should, and it's still there. Pressing again the close Button, FormClose is executed again, and the same dialog is shown. No way to close the form, unless you press Cancel on the error dialog.
Why FormClose doesn't close the form in presence of an error (handled by a try..finally) when writing the ini file?
Launching from command line, you have the same behavior, and an additional information when you select Cancel:

WARNING: TLCLComponent.Destroy with LCLRefCount>0. Hint: Maybe the component is processing an event?

Try..except situation, from IDE and Debugger.
Press the Close button. Debugger Notification. Pressing Continue my message dialog is shown. Pressing OK on my dialog the form is closed and the program is terminated.
Launching from command line, no console messages.

My conclusion is that one can't properly handle INI files with just a try..finally construct, as all examples show, because a possible error will propagate outside the construct, with unpredictable effects (in this case the FormClose procedure doesn't properly complete).

Giuliano

Giuliano Colla wrote

My conclusion is that one can't properly handle INI files with just a try..finally construct, as all examples show, because a possible error will propagate outside the construct, with unpredictable effects (in this case the FormClose procedure doesn't properly complete).

Hello,

Finally always propagate the exception bubbling up looking for other exception handler, and that's its job, in comparation to except handler which "eat" the exception and if you need to propagate it again you must "Raise" in the exception handler.

Mark Morgan Lloyd wrote

I admit that I was slightly trolling there, since Giuliano was complaining about exceptions that he wasn't seeing (because, it turns out, he wasn't catching them).

You catch an exception if you can handle it. If a user for some reasons has write-protected a configuration file, there's nothing the application can do about it. One can usually rely on the system default exception handler to show the error message. If it doesn't happen, then there's something wrong somewhere.

Giuliano

[snip]

One may test two conditions: with try..finally and with try..except. Even with this minimal sheds some light on the matter.

It indeed does, but not as you think.

The problem is not INI files, but more how OnClose is handled. OnClose is called whenever the user tries to close the form and if an assigned event handler raises an exception it seems that the calling code assumes that the form should not be closed. In my example I used OnDestroy which seems to be handled more gracefully.

I'd suggest you to ask on the Lazarus list whether it is intentionally that you can't close a form of which the OnClose event handler raises an exception.

Regards,
Sven

This is also so in Delphi. Just tested.

Michael.

Giuliano Colla hat am 12. Februar 2013 um 13:29
geschrieben:

The LCL has a default exception handler, so that the application notifies the user, that the application has a bug instead of simply crashing and vanishing silently.
The programmr is reponsible to handle exceptions, show the user error messages and give the user choices (e.g. ignore, retry).
BTW, the Ini.Free writes to disk, so it needs the try..except, the WriteValue calls do not need the try..except.

Mattias

That depends.
if you disabled write caching, every write call will update the file.

Michael.

Yes, but the point that I'm trying to get across is that the earlier you make sure that you've got full access to the files (i.e. all directories in the path exist, the file either exists and is writable or you create it from a template) the easier you make life for yourself. If having the file is absolutely essential then check for it before you even open the main form and bomb if it's obvious that there's a problem.

That's what I was relying on. There's nothing the application can do if a user has write-protected the configuration files, or if there's a disk error while writing (I didn't think of a non existing .config/, because I've always found it there, before the last episode). So I was just happy with a system error, which in this case fails to show.

As an additional information:
in FormClose

ini := TIniFile.Create(BadAppConfigFileName);
try
   ini.WriteWhatever(...
   ...
except
   on E: Exception do Application.ShowException(E);
end;

shows the error only on the console, but no visual dialog. While

   on E: Exception do MessageDlg(E.Message,mtError,[mbOk],0);

shows a proper message dialog.

Maybe this discussion should be moved to Lazarus list?

Giuliano

That's the point. The configuration file in this application is useful but not essential. It holds fine calibration data for a set of remote sensors. If you can't read it, you must click again on the "Calibrate" button a number of times, to display more precise values. One could even chose to write-protect it, to avoid overwriting a good calibration. It's not a DataBase application where if you don't set the right hostname, user and password you can't do a thing.
That's why I would have been perfectly happy with a default system error box telling "write error", or whatever, just to let know that the data have not been saved.
What happens on the contrary is that, in case of failure, no error is shown, and, what's more annoying, that the form can't be closed, which can be quite confusing for a normal user.

Giuliano

apples and oranges... one is an anti-virus product and the other a firewall ;)

Does it brew coffee and vacuum the floor as well? >:-}

Ralf ;-)

No one forbids those growing apple trees to grow orange trees as well: http://www.zonealarm.com/security/en-us/zonealarm-free-antivirus-firewall.htm

Regards,
Sven

I so hope not...

Note: If anyone wants to answer to this "anti virus" thread that was off topic to begin with, we should move over to fpc-other.

Regards,
Sven

I'm not sure if somebody else mentioned this already but I have the feeling that Giuliano thinks that he has to decide to use try-finally OR try-except but there are situations where you need both. One of the criteria is if the object is local or global since for a global object it is likely that you place the .Free (or for a global object better FreeAndNil()) into a destructor or finalization section (which means that you have to stop the program in case of an error and not continue showing just a dialog) so you'll need only the try-except if you want to catch the exception. In case of a local object you'll ALWAYS have the lines

Obj := Class.Create;
try
  // do some code
finally
  Obj.Free;
end;

otherwise you'll end up with memory leaks.
If you want to catch the exception in this situation you'll put try-except extra around try-finally so you'll end up with

Obj := Class.Create;
try
  try
    // do some code
  finally
    Obj.Free;
  end;
except
  writeln('We have a problem);
  halt the program or reraise the exception or raise a new one
end;

Regards

Michael

I tried to make that point but I'm not sure the message reached its mark.

It's possibly unfortunate that Borland chose to use "try" for both constructs, rather than having distinct try-except-end and start-finally-end forms.

To developers:
How would a generalized/packed construct like

try
[code block]
finally
[code block]
except
[code block]
end;

(in other words: a try-*-end construct where * can be 'finally', or 'except', or BOTH.)

fit into Pascal philosophy?
Advantages is mainly:
- one less indent level ('oh, I need try-except around all THAT, bugger.') to care about; (yeah, even with all the good tools to manage the code, it stings, that the two have to be separately declared and one needs to remember that...)

Would it be very complicated?

Lukasz

My apologies, I should have added [OT] to my reply.

Lukasz

I think this complicates things unduly. As it is case-else and then-else are about the only places where there is this sort of composite block, and finally-except-end is even uglier since parts of all three blocks may be executed.

Other languages did the same, so I don't see the problem here...

Regards,
Sven

or what about

try
[code block]
except
[code block]
finally
[code block]
end;

In any case the semantics need to be defined correctly. E.g. in the first case what happens if an exception is raised inside the "finally" block? In the second case what happens if an exception is raised inside the "except" block? Do they work like nested "try try except end finally end" or "try try finally end except end" blocks or do they have the same rules?

Keep this FAQ entry in mind when suggesting features: http://www.freepascal.org/faq.var#extensionselect

(Note: I personally would have nothing against such a feature, but the details need to be defined!)

I don't know how complicated it would be (we now also need to take into account Win64 SEH and possibly in the future also Win32 and WinCE SEH), but it should at least be doable...

Regards,
Sven

With the traditional construct there could be another place that poses a potential pitfall:

[use fixed width font to see below comfortably]

try vs try
try [...]
[...] finally
finally [...]
[...] except
end; [...]

  //(*1*)		end;

except
[...]
end;

Using the composite one, protects the user of this construct from putting code into *1* place
where it may not be executed if exception happens in the try..finally block;

I'm not saying, this is not sometimes that one wants, but (at least in my case, YMMV) putting code in *1 is a result of a thinko/PEBKAC. There, this I think is another (better?) argument in favor of composite try...finally...except...end;

I still think the latter (single-level, composite) is nice to have. And acknowledges that the two are in effect used together most of the time.

And lets one to easily add a 'finally' or 'except' section if need arises, without loosing any code to *1* black hole.

Regards,
Lukasz

Sven Barth wrote

On 13.02.2013 10:11, Lukasz Sokol wrote:

[...]

I'd imagine it should work exactly like try try ... finally ... end; except ... end; so if exception happens in 'finally' block, 'except' is executed, if no exception happens anywhere, only 'finally' happens.

Keep this FAQ entry in mind when suggesting features: http://www.freepascal.org/faq.var#extensionselect

Nice, thanks!

I was imagining something in or just before the parser, detecting the 'composite' construct and 'mechanically' (string for string even) substituting it for two-level traditional, putting the 3 code blocks into the proper places. No ifs, no buts, same semantics, just flattened.
Write once, compile everywhere ;) Unfortunately I don't have enough compiler knowledge to even imagine where this could be happening.

That would have all cases covered then...?

Lukasz

Well I guess unless someone is ready to have a precedent of relaxing rule 2 here 'Anything that is only a shorter notation does not apply ...' this won't happen :(

But it's not just 'shorter', it's relatively 'safer' too, and doesn't forbid anybody from using traditional constructs either, at all, if they wish so/need so.

Lukasz

Precedent: case of String

If the other reasons are compelling enough or someone even provides a patch for the feature I personally see no problem. Of course the opinions of the other core developers matter as well...

But it's not just 'shorter', it's relatively 'safer' too, and doesn't forbid anybody from using traditional constructs either, at all, if they wish so/need so.

That's for example a reason that speaks for the feature.

Regards,
Sven

Sven Barth wrote

On 13.02.2013 12:41, Lukasz Sokol wrote:

[...]

Thanks, your acceptance made me shed a tear of joy and made my day brighter :)

Lukasz

It's mostly because I want that feature for a long time myself. :)

You'll nevertheless have to convince at least a few more core devs that this feature would be useful or/and implement it yourself... [mostly because I'm currently busy with other projects, either FPC related or not ^^]

Regards,
Sven

Sven Barth ha scritto:

Python provides the following:
try
[code block]
except
[code block]
else
[code block]
finally
[code block]
end;

which can be used in any reasonable combination: just try..except, just try..finally just try..except..else etc.

The except..else is a very useful construct, because it provides a path to a code block to execute only if there were no previous errors. This wouldn't break any existing applications, just add very useful features.

What fpc developers think about that?

Giuliano

"Else" is used in Except already to select between various classes:

try
   ..
except
   on E : MyType do
     begin
     end
else
   // exception is not MyType
end;

so that is a problem.

I see no problem in adding finally, if you define carefully when it is executed.

Michael.

I second that, we don't need no pythonisms here ;) (or is it pythonosis?)

I'd rephrase what I discussed with Sven Barth:

this construct (the packed try.(1).finally.(2).except.(3).end;) - a packed/condensed version -

- should be semantically EXACT to try.(HOLE1).try.(1).finally.(2).end;.(HOLE2).except.(3).end;
  Legend:
  (1,2,3): code blocks
  (HOLE1) possibility for the try..finally..end block to be skipped if exception happens here
  (HOLE2) possibility for code to be skipped if exception happens in the try...finally...end; block

- except, it will not have the HOLEs. Because [maybe some statistics needed here] this is probably the main (most advocated ?) use of them both at the same time.

- It shall allow any constructs that are allowed in traditional (1,2,3) code blocks to be used exactly the same way with the same meaning.

The new way will:

-flatten the source code (one indent level less)
-shorten the source code (one 'try' and one 'end;' less) -provide automatic protection from falling into the (HOLEs). (thinkos and PEBKACs protection) (this is most important here I think).

Note that I'm not advocating for this to replace the traditional constructs; Also I accept, that HOLEs may be perfectly justified code. But if someone needs them, they probably know what they are doing.

Hope this makes sense,
Lukasz

Michael Van Canneyt ha scritto:

I didn't think of that, because I never use it. It would be nice to use it the other way, but I understand it would break existing code. A different keyword like nonexcept?

I see no problem in adding finally, if you define carefully when it is executed.

IMHO the try..except..finally construct should provide exactly the same
functionality as a nested
try
   try
   [code]
   except
   [code]
   end;
finally
[code]
end;

i.e. it should be executed whatever happened in the sections between try and finally (except if somewhere there was an Application.terminate!). Any exception not handled shoud be re-raised after the finally block has been executed.
The advantage would be to get rid of one level of nesting, making the code more readable and less error prone.
This would guarantee both proper error handling and freeing of global resources, even in presence of errors.

Giuliano

Lukasz Sokol ha scritto:

I would be in favor of try..except..finally, as opposed to try..finally..except, because I believe it better to handle exceptions before, and clean-up after. Exception handling doesn't consist only in showing an error box, it may involve also providing a remedy to some errors, and hopefully passing to finally a cleaner situation to handle. I'm ready to change advise if proved wrong.

If one is left free not to use the new construct, he can use the old one, and put in as many holes as he want.
After all one is free to insert anywhere in the code a few assembler lines and to break anything fpc developers had in mind..

Giuliano

I don't see the point of a "else" or "nonexcept" branch. If I want code to be executed in the case that no exception happened (e.g. a Commit for a database transaction) then I put in the block started by the "try"... One might argue that an additional branch would increase readability, but I personally don't see a real use for it...

Application.Terminate is not special. It merely sets a "FTerminated" to True which will trigger a leave of the application main loop once the event handler returns. The only function that would be special in that regard is "Halt", but that already works that way that any handlers are skipped.

Regards,
Sven

As the proposed construct is a "mere" syntactic sugar anyway I see no problem in allowing both cases (after all you can do both ways with the original constructs as well)...

Regards,
Sven

It is subtle, I agree, but in

try
  try
  [code]
  finally
  [code]
  end;
except
[code]
end; (my version)

I can catch the errors of either normal and finally code; I assume that try...finally...end is the more important; it is try...finally.. end that has to go without errors, or else one will be reported; if no exception happens, except is omitted; the except section in this case, can catch anything and its job is to decide whether to allow further execution, show/log a message or bail out. (also AFAICS this way it is recommended to be used in most snippets)

In your case (try..finally...end being the outer), what if it may not be possible to free the global resources (e.g. if you .Created a TStringList, and in finally you want to .Free it, but say, its pointer got corrupted or something free'd it already, and you can't - you'll get an AV which you'd have to catch in another try..except..end to properly show user a message? Or if something else that has to be done in finally section, throws?)

Kind Regards,
Lukasz

Lukasz Sokol ha scritto:

The purpose of the try..except construct is to allow the developer to put some remedy in case of errors, instead of just aborting the program. The purpose of the try..finally construct is to guarantee some actions even if the program is going to be aborted.
The new constructs would provide both options at the same time. But I'm afraid that there's not a "better solution" good for all situation. It depends on what one tries to do in the try.. clause, on which errors one choses to deal with on the except clause, and the likelihood of recoverable errors in the finally clause.
Allowing both constructs, (try..except..finally and try..finally..except) as Sven suggested, would most likely cover a broader range of cases.
Then, sometimes someone will still need nested try..whatever, to deal with situations which do really require the HOLES you mentioned.

Giuliano

Sven Barth ha scritto:

You're right. At first glance it appeared to me a good idea, but actually it wouldn't provide anything useful.

At first I thought, that having 2 versions of this construct, would be overkill; and whether it is intuitive enough for everybody that the last section will be the 'outside', it's hard to say without others input;

For myself, the more intuitive construct is, to do try, then finally, if no exception anywhere then skip except, otherwise (whether it happened in try or finally) execute the exception handling code. Which equals the tandem construct with except on outside.

But now, I think it actually would be a good thing to have the other too;

Whether the order of the inside/outside sections should be selectable by order of keywords in the flattened construct, is still debatable; I think if majority agrees that this is intuitive enough,

Main reason is to close the HOLEs, secondary, flatten the code and shorten it down even if only a bit.

Maybe he one and true answer for all of the above would be to have:

try     vs		try
  try			except
    try			finally
    except		except
    end;		end;
  finally
  end;
except
end;

so with except being optionally allowed either side of 'finally' ?

Lukasz

Giuliano

try
  try
  except
  end;
finally
  try
  except
  end;
end;

If this is what is really desired, is this a good construct?

try
  ...
except
  ...
finally
  ...
except
  ...
end;

I don't care for the meaning of "except" looking to be contextual, but is it really? Reading that, to me it looks mostly predictable how the logic would have to work. I guess the only question is whether the finally code is executed if the try code has an exception. If this isn't desired, could a "break" be used in the first except to cause the finally to be skipped?

Jeff.

Does "break" do it in traditional try..finally..end? This new construct shall be no different.

That's actually why I opt for just try..finally...except..end; too - because an exception in the 'finally' section, if it was the outer, could have unpredictable consequences; with 'except' being outer, at least your program will have a chance to still run and exit (more or less) gracefully.

Lukasz

Jeff.

The idea for the construct is to replace (if I take your last example) the following construct:

=== example begin ===

try
   try
     try
     except
     end;
   finally
   end;
except
end;

=== example end ===

The variant

=== example begin ===

try
finally
except
end;

=== example end ===

would replace

=== example begin ===

try
   try
   finally
   end;
except
end;

=== example end ===

and

=== example begin ===

try
except
finally
end;

=== example end ===

would replace

=== example begin ===

try
   try
   except
   end;
finally
end;

=== example end ===

These are the most common usages of nested try...finally/except blocks and thus can increase the readability. Thus no change in semantics, only in "formatting".

Regards,
Sven