[macro] no private field control for exprs generated with macro

[nadako]

I feel like this should fail to compile with Cannot access private field id error, shouldn't it?

class A {
    var id:Int;

    public function new() {}
}

class Main {
    static function main() {
        var a = new A();
        f(a);
    }

    static macro function f(v) {
        return macro $v.id = 5;
    }
}

[Simn]

That's the @:privateAccess feature which was added for expressions returned by macros to deal with... some problem that I don't recall right now because it's christmas and you should stop opening issues. :P

[nadako]

Haha, we celebrate christmas two weeks later here so I had no idea :-P Anyway, it looks quite sloppy to me having @:privateAccess by default in all macros, because when you actually need those checks (like in my case), you have to do them by hand with complex macro code. Maybe we could at least have @:nonPrivateAccess?

[ncannasse]

That's indeed an issue, I think we should keep it open :)

[Simn]

We designed it like this on purpose, but I really don't remember why. Chances are @back2dos was involved.

[back2dos]

I think this came up because of how static extension macros worked, i.e. round tripping expressions through the typer, which meant that for example a field read access would become a call to the private getter, so when you returned that back, you got type errors from the private access. However because since 3.1.0 the expressions are handled differently I would speculate that this is now obsolete.

If this is was the only reason, then the @:privateAccess should only have been added for static extension macros and everything would function correctly. So there may be other causes.

Personally, I always add @:privateAccess by hand when required, because I do most of my twisted experiments in build macros anyway. So my suggestion is to get rid of it and see if anyone notices ;)

[Simn]

Static extensions were the only reason I could think of too, and I agree with your conclusion. Thanks for remembering!

[Simn]

I wonder if we should revert this. It seems to cause a bit of confusion because it's hard to tell where the problem comes from.

[nadako]

Could we then add @:noPrivateAccess or something to enable access checks explicitly?

[Simn]

I'm not sure what that would actually improve...

[nadako]

I remember writing some syntax-sugar macros where I didn't want to loose access control to keep things error-proof, that's why I reported this issue. I don't remember the exact case, but I'm sure there are plenty of use cases.

[Simn]

Yeah but that shouldn't be about specific types. If anything we should have a -D flag for this.

We could go for a general -D transition flag which enables behavior that we are plan to make the default in the future. That could also be used for the is problem.

[nadako]

Actually, I remember you writing a macro that allows to do:

myObj.with(
    a = 1,
    doStuff()
);

which is then translated to

myObj.a = 1;
myObj.doStuff();

It would make perfect sense to keep access restrictions in such case.

[nadako]

I don't follow. Do you want to keep old "everything is public within a macro" behaviour, but hide it behind -D transition?

[nadako]

(also I'm worried about libraries defining that flag with extraParams.hxml)

[Simn]

I meant the opposite. Do I really have to worry for every single flag about libraries possibly defining them? There's always the option to tell such a library author that he's being an idiot.

Anyway, I have reverted the change, feel free to add whatever you feel is right but please keep this as the default.

[DanielUranga]

Maybe improving the error report suggesting to use "@:privateAccess". Oh too late, not the default now.

[nadako]

Just after I pushed the @:noPrivateAccess I thought about the awkward case like @:privateAccess someMacroThatDoesNoPrivateAccess(), but that's probably for macro authors to deal with.

[nadako]

I still think it's weird to implicitly lose access control when calling a macro by default. Macros are supposed to be ast transformations and having that side effect is unexpected.

[nadako]

By the way, I noticed that @:privateAccess from macro doesn't get applied in interpreter mode.

E.g. this will fail with Cannot access private field a when run with --run Main

class A {
    static var a:Int;
}

class Main {
    static function main() {
        m();
    }

    static macro function m() return macro A.a;
}

[Simn]

I don't disagree but it still presents a problem that is really hard to track down, especially for mere users of a macro (library).

[back2dos]

Firstly, I think removing this auto-generated @:privateAccess is the right
move. I was the one who requested it for the reasons already explained.
It's not like you couldn't work around it at the time either (this was
before EMeta):
https://github.com/back2dos/tinkerbell/blob/cf08e70d2f6c68f8d70fe1d1afd6f2ab306cf392/src/tink/macro/tools/ExprTools.hx#L37

Given the circumstances, I thought it would be the best thing we could
offer other library authors. The circumstances have changed, and now it
doesn't make sense to do this anymore.

Secondly, macro libraries are still very exotic and largely only used by
early adopters. Now is the best time to get it right, I think. Haxe 4 would
be the next best opportunity. But given how few people it would affect
right now, I think now is a better occasion.

From Haxe 3 on, every release broke something about macros. And I think it
was worth it. I don't think this will create more problems than the changes
to static extension macros did. That really made quite a lot of stuff blow
up in my face I can tell you. If you have
someObj.someField.bindExpr(theExpr) and you get an error around
someObj.someField telling you "field access expected", then THAT is what
I'd call a confusing error message ;)

The @:noPrivateAccess thing is still pretty useful I think, but
auto-@:privateAccess
should be discontinued, if you ask me. I think it's fair enough to expect
the library maintainers to deal with it.

On Thu, Mar 26, 2015 at 5:12 PM, Simon Krajewski notifications@github.com
wrote:

I don't disagree but it still presents a problem that is really hard to
track down, especially for mere users of a macro (library).

—
Reply to this email directly or view it on GitHub
#3714 (comment)
.

[ncannasse]

I don't agree with the revert of the change : I think that we should not add @:privateAccess and let the macro authors deal with the change, which was a "bug" IMHO to let accessing all privates as soon as you had an expr returned from macro.

[Simn]

It was not a bug, it was a deliberate implementation in the past and just because nobody remembers the exact reasoning doesn't mean that we can just discard it. There have been multiple reports of it in the RC phase and most users who are affected by this cannot fix it by themselves. This is very frustrating and adds to the perception of Haxe being unstable.

[ncannasse]

Yes I understand how it was not a bug, but if we are planning to change that at some point (and we do), the the sooner the better. Haxe is growing, and the number of users/libraries as well. So let's make the change asap and help library authors to fix their code, or else it will be even harder and will affect even more users to do that for the next version.

[Simn]

I lack the energy to discuss this further. I have reverted the change.

[nadako]

That was unexpected, but I'm certainly fine with it :-) I mentioned the change in https://github.com/HaxeFoundation/haxe/wiki/Breaking-changes-in-Haxe-3.2.0#macro.