View Issue Details

IDProjectCategoryView StatusLast Update
0023032LazarusIDEpublic2019-09-11 13:07
ReporterAlexander StrokachAssigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
Status confirmedResolutionreopened 
PlatformOSWinOS VersionXP x32
Product Version1.0.0Product Build 
Target VersionFixed in Version 
Summary0023032: IDE does not warn if eventhandler has wrong signature
Description  TForm1 = class(TForm)
    Button1: TButton;
    Button2: TButton;
    procedure Button1Click(Sender: TObject; flag: boolean = false);
    procedure Button2Click(Sender: TObject);
    procedure FormActivate(Sender: TObject);
  private
    { private declarations }
  public
    { public declarations }
  end;

var
  Form1: TForm1;

implementation

{$R *.lfm}

{ TForm1 }

procedure TForm1.Button1Click(Sender: TObject; flag: boolean = false);
begin
  ShowMessage(BoolToStr(flag, 'true', 'false'));
end;

procedure TForm1.Button2Click(Sender: TObject);
begin
  Button1Click(Form1);
end;

procedure TForm1.FormActivate(Sender: TObject);
begin
  Button1.Click; // true
  Button2.Click; // false
end;
TagsNo tags attached.
Fixed in Revision
LazTarget-
Widgetset
Attached Files

Relationships

has duplicate 0024999 resolvedMichael Van Canneyt Eventhandler Parameters Changed between Versions 

Activities

Sven Barth

2012-10-02 15:48

manager   ~0062829

This can not work, because TButton.OnClick is defined as TNotifyEvent which is defined as "procedure(Sender: TObject) of object". So the compiler can not know that it needs to pass an additional parameter. If you manually do a "Button1.OnClick := @Button1Click" you'll even get a compile time error.
That you don't get an error at runtime is only because it can not be checked whether the arguments of a method pointer and a method name read from the DFM are equivalent. Also you don't get an error out of pure luck: If you'd use a more complicated type (like String) you'd be more likely to get an access violation or something like that as the defaulting parameter will contain garbage or even part of your local variables.

Regards,
Sven

Vincent Snijders

2012-10-02 17:06

manager   ~0062830

As explained by Sven.

Anton

2012-10-02 19:43

reporter   ~0062832

Delphi IDE checks this:
The Button1Click method referenced by Button1.OnClick has an incompatible parameter list. Remove the reference? [Yes No Cancel Help]

Bart Broersma

2012-10-03 16:07

developer   ~0062858

@Anton: I cannot even get the IDE (OI) to let me select such a method as an OnClick event.
I can only get it to comile if I manually edit the .lfm file.

Anton

2012-10-03 20:01

reporter   ~0062861

Last edited: 2012-10-03 20:02

I mean that it is not compiler but IDE who should make this check, because IDE knows that method became not-compatible.

Bart Broersma

2012-10-03 21:23

developer   ~0062865

Last edited: 2012-10-03 21:26

Yes, what the reporter wanted to do is (very) wrong, but since the OnClick method is not a TNotifyEvent an error message should be presented (either by compiler or by IDE) when compiling or loading.
The IDE happily accepts even the most ridiculous methods like:

procedure WrongClick(var x,y: Integer; out z: String; var W: WideString);
(See attached example)

Ofcourse at runtime this will crash as predicted, but it should never have been allowed to be built in the first place.

2012-10-03 21:25

 

wrongclick.zip (6,011 bytes)

Vincent Snijders

2012-10-03 21:30

manager   ~0062866

I still is this is a 'won't fix' or 'no change required', depending how the OP created this error. So I un-assign the issue.

Bart Broersma

2012-10-03 22:52

developer   ~0062868

I admit, if you are clever enough to manually edit the lfm, you should know what you are doing. But it might have been an accident, perhaps a wrong merge of some kind.

Bart Broersma

2012-10-09 10:50

developer   ~0063020

Updated summary to reflect actual problem.

2012-10-13 11:26

 

Bart Broersma

2012-10-13 11:28

developer   ~0063117

Last edited: 2012-10-13 11:30

> I still is this is a 'won't fix' or 'no change required', depending how the OP
> created this error.

Well, actually it's quite easy. No need to edit the lfm file at all (as i suspected at first).

Steps to reproduce:
1. place a button on the form
2. in OI create default OnClick eventhandler (by double-clicking on the entry)
3. change "procedure Button1Click(Sender: TObject)" into anything that is not a TNotifyEvent.
4. Save, build, run.
5. No compile-time error, no error when loading project in IDE.
6. Runtime error on run (as expected)

Step 3 is something you and me would never do, since we know an OnClick event must have the TNotifyEvent signature.

A novice however might think he could extend the OnClick handler like OP did.
It could be argued that in this case the "novice" should have known better, but then he would not have been a novice anymore.
The runtime error that follows is then hard to debug. Only if you look in OI and see that it has a wrongfull entry for an OnClick handler you know what the problem is. When the OnClick handler has a name that does not have the word "click" in it, changes are you will never find the error just by looking at the code.

Delpi (3 Pro) does not even allow compiling and raises an "incorrect method declaration" error when trying to compile (see attached screenshot).

2012-10-13 20:40

 

d7_error_message.png (8,497 bytes)
d7_error_message.png (8,497 bytes)

Bart Broersma

2016-02-27 17:07

developer   ~0090408

Last edited: 2016-02-28 11:15

View 2 revisions

From 0029720
> You must have edited this yourself.
Yes, because now I get again this warning when compiling:
Roughness.pas(59,29) Verbose: Parameter "Sender" not used

People do stupid things, then get hard to understand bugs (and complain to us).

And 0024999. Rather annoying (and hard to find) if all you did was "svn up".

What do we need in order to fix this, either at the stage of compilation and/or at the stage of loading the form into the Lazarus IDE?

Sven Barth

2019-09-11 09:27

manager   ~0118030

Another valid use case was mentioned on the forum ( https://forum.lazarus.freepascal.org/index.php/topic,46692.msg333254.html#msg333254 ): Namely changing a component's type using "Change Class".

Issue History

Date Modified Username Field Change
2012-10-01 06:10 Alexander Strokach New Issue
2012-10-02 15:48 Sven Barth Note Added: 0062829
2012-10-02 17:06 Vincent Snijders LazTarget => -
2012-10-02 17:06 Vincent Snijders Status new => resolved
2012-10-02 17:06 Vincent Snijders Resolution open => no change required
2012-10-02 17:06 Vincent Snijders Assigned To => Vincent Snijders
2012-10-02 17:06 Vincent Snijders Note Added: 0062830
2012-10-02 19:43 Anton Note Added: 0062832
2012-10-03 13:16 Vincent Snijders Project Patches => Lazarus
2012-10-03 16:07 Bart Broersma Note Added: 0062858
2012-10-03 20:01 Anton Note Added: 0062861
2012-10-03 20:02 Anton Note Edited: 0062861
2012-10-03 21:23 Bart Broersma Status resolved => assigned
2012-10-03 21:23 Bart Broersma Resolution no change required => reopened
2012-10-03 21:23 Bart Broersma Note Added: 0062865
2012-10-03 21:25 Bart Broersma File Added: wrongclick.zip
2012-10-03 21:26 Bart Broersma Note Edited: 0062865
2012-10-03 21:26 Bart Broersma Note Edited: 0062865
2012-10-03 21:30 Vincent Snijders Note Added: 0062866
2012-10-03 21:30 Vincent Snijders Status assigned => acknowledged
2012-10-03 22:52 Bart Broersma Note Added: 0062868
2012-10-09 08:35 Vincent Snijders Assigned To Vincent Snijders =>
2012-10-09 10:50 Bart Broersma Note Added: 0063020
2012-10-09 10:50 Bart Broersma Category Other => IDE
2012-10-09 10:50 Bart Broersma Summary The default value is not assigned in the procedure => IDE does not warn if eventhandler (in lfm) has wrong signature
2012-10-13 11:26 Bart Broersma File Added: incorrect_method_declaration_error.png
2012-10-13 11:28 Bart Broersma Note Added: 0063117
2012-10-13 11:29 Bart Broersma Summary IDE does not warn if eventhandler (in lfm) has wrong signature => IDE does not warn if eventhandler has wrong signature
2012-10-13 11:30 Bart Broersma Note Edited: 0063117
2012-10-13 11:30 Bart Broersma Note Edited: 0063117
2012-10-13 20:40 Anton File Added: d7_error_message.png
2016-02-08 16:52 Bart Broersma Relationship added related to 0024999
2016-02-13 11:47 Michael Van Canneyt Relationship deleted related to 0024999
2016-02-13 11:47 Michael Van Canneyt Relationship added has duplicate 0024999
2016-02-27 17:07 Bart Broersma Note Added: 0090408
2016-02-28 11:15 Bart Broersma Note Edited: 0090408 View Revisions
2019-09-11 09:27 Sven Barth Note Added: 0118030
2019-09-11 13:07 Bart Broersma Status acknowledged => confirmed