nielsAD / lape

Scripting engine with Pascal-like syntax for FPC and Delphi
118 stars 28 forks source link

Integer value to boolean param #63

Closed riwu closed 8 years ago

riwu commented 8 years ago

Not sure if it's already fixed:

procedure a(x: Boolean);
begin
end;

begin
  a(2); //works even though integer != boolean (doesnt work in fpc)
end.

Similar to https://villavu.com/forum/showthread.php?t=110843

slackydev commented 8 years ago

In the version of lape you are testing, booleans are integers pretty much to the core. This behavior has however been changed with our recent boolean fixes, where booleans now have their own evaluation methods, and don't ever rely in integer-evaluation.

Currently this is the result of assigning bool to int, and vice versa:


I do however think this is a feature we can consider supporting again, as it's kinda nice to have such a cast implied. But there are some "issues" with supporting it, as it allows people to easily write flawed code, which is seen many places: ¤ https://github.com/SRL/SRL-6/blob/master/lib/utilities/color.simba#L718 ¤ https://github.com/SRL/SRL-6/blob/master/lib/utilities/color.simba#L767 ¤ https://github.com/SRL/SRL-6/blob/master/lib/interfaces/targetinfo.simba#L149 etc... the list isn't that long.. But I think the real sinner is the irrational precedence of the and-operator. But eyy.. it has two uses in pascal.. soooo.

riwu commented 8 years ago

Yeah i agree that boolean operators having higher precedence than comparison operators is weird, but probably because in fpc bitwise operators (which usually has higher precedence than comparison) share the same syntax as boolean operators, so it might not be a good idea to deviate from FPC.

Converting integer to boolean is trivial (someInt <> 0) and more readable/flexible so i don't think that's necessary at all, esp since like you mentioned, it causes hard-to-debug (for ppl not aware of the abnormal precedence) bugs.

riwu commented 8 years ago
type
  enum = (a, b, c);

procedure f(x: enum);
begin
end;

begin
  f(1);
end.

Should not allow int to be passed as enum args too. If someone does that, he's likely to have made a mistake. Stricter param type check (like in FPC) will prevent many flawed codes and allows greater flexibility in method overloading.

slackydev commented 8 years ago

I have never seen that being an issue. And I do like that implicit cast, I feel it often keeps my code cleaner. So if it was up to me I would not see it worth the work, since I don't see an issue with it.

riwu commented 8 years ago

Why use an integer argument if the parameter is an enum? Any example of how it keeps the code cleaner?

slackydev commented 8 years ago

A change like this would affect other things as well, as we would redefine what enum is compatible with (making it incompatible with Integer), so: Arr[ltMethod] := someValue; would be forced to be: Arr[Ord(ltMethod)] := someValue; Which is less readable, more notable with a longer more complex index, eg: item := Arr[Ord(self.GetCurrentTab())] versus item := Arr[self.GetCurrentTab()]

Most of all I think this is a made up issue. I've never seen, nor heard of a case where someone has managed to mess up the way you describe. It would be a hacky to only disallow calls, without affecting the usage shown above .

riwu commented 8 years ago

self.GetCurrentTab()/ltMethod should not return an enum if it's meant to be use as array index? Why not just return integer instead?

Also i think codes should not rely on the ordering of enum. One of the main benefits of using enum rather than const is that you can change the ordering/add new const without breaking the code. Enum params are generally handled with a case statement.

Preventable mistakes (maybe not a good example but you get the idea)

procedure Mouse(x, y: Integer; clicktype: TClickType = MOUSE_NONE; offsetX, offsetY: Integer = 0);
begin
end;

begin
  Mouse(100, 100, 2, 2); //missed out the clicktype param but no warning
end.
nielsAD commented 8 years ago

I agree. These checks could be a bit more strict. The array index operation should not be in danger as that works with all ordinal types (and does not use cast to integer).