HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.17k stars 654 forks source link

Abstract weird @:op not throwing error #11237

Closed filt3rek closed 1 year ago

filt3rek commented 1 year ago

Hej,

@:op(foo(A, B).bar) function bar(){} doesn't throw any error. I mean you can put almost anything inside and it won't throw any error. It would be helpful if it would throw one. @Apprentice-Alchemist suggested that :

| (Meta.Op, _, _) :: _ ->
  raise_typing_error ("Invalid @:op expresssions, should be an operator or a call") cf.cf_pos

Here : https://github.com/HaxeFoundation/haxe/blob/baee98f4be27d8c4b63620535e1d20284d831cab/src/typing/typeloadFields.ml#L1175

Could it be done and if yes, can you add this if you please ?

Thanks

Simn commented 1 year ago

I actually do get an error for this: Field type of resolve must be Int -> String -> T. This is because the expression is processed as EField. While this is a bit silly, I don't want to add too many checks here.

filt3rek commented 1 year ago

@Simn Thx for your answer, but maybe we misunderstood I mean something like this doesn't throw any error : https://try.haxe.org/#B3FB3586 (with nightly Haxe)

abstract A(Int) from Int to Int {
  @:op(foo(A, B).bar) function add(s:String) {} // no error
  @:op(sdfgsdrgsdrgergsergserfgse) function sub(s:String) {} // same : no error
}
class Test {
    static function main() {
        var a:A = 5;
    }
}

or @:op( sdfgsdrgsdrgergsergserfgse) doesn't throw any error

But maybe I don't understand and you have already made a fix ... idk Thx anyway

Simn commented 1 year ago

The @:op(foo(A, B).bar) doesn't error because it works. The exact nature of the expression is irrelevant, the compiler only cares about it being an EField expression.

filt3rek commented 1 year ago

Yes I understand that, it belongs more to how all meta are handled in Haxe, but in the special case of @:op I find it brings confusion because one would that Haxe complain if it's not a valid operation. It gives an error when doing that @:op(A=B), so why not in other cases ? :

abstract A(Int) from Int to Int {
    @:op(A = B) function assign(s:String) {} // "Assignment overloading is not supported"
}
class Test {
    static function main() {
        var a:A = 5;
    }
}

I personally don't care about that because I begin to understand Haxe for years of using it now but I'm thinking about new Haxe users, I sometimes bring some weird cases in order to help Haxe to be more friendly for new users that's all 😊

Simn commented 1 year ago

Don't worry about that, the chance of a new user being more confused than you is quite low.

But I'll try one more time:

  1. @:op(foo(A, B).bar) works the same way as @:op(A.bar), it's an EField expression and the compiler understands it as a @:resolve method.
  2. @:op(abc) and any other not-supported expression now errors after my commit, as you can see in the test I've added for this.