Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP global namespace again #5071

Merged
merged 2 commits into from
Apr 7, 2016
Merged

Conversation

mockey
Copy link
Contributor

@mockey mockey commented Apr 7, 2016

  • add specific @:phpGlobal metadata to get around the issues with @:native
  • remove the previous attempt of using @:native
  • add @:phpConstant prefix from previous PR
  • avoid undesired var renames with s_ident for externs in global php namespace

remove the previous attempt of using @:native
add @:phpConstant prefix from previous PR
@mockey
Copy link
Contributor Author

mockey commented Apr 7, 2016

So now you can write:

@:phpGlobal extern class Var {
  @:native("isset")
  static function isSet(val:Dynamic):Bool;

  @:native("\\var_dump")
  static function dump(val:Dynamic):Void;
}

@:phpGlobal
@:phpConstants
extern class MagicConstant {
  @:native("__FUNCTION__")
  static var FUNCTION:String;
}

@:phpGlobal
@:phpConstants("\\SOAP_SSL_METHOD_")
@:enum extern abstract SoapSslMethod(Int) {
  var TLS;
  var SSLv2;
  var SSLv3;
  var SSLv23;
}

@Simn Simn merged commit 6a24359 into HaxeFoundation:development Apr 7, 2016
@Simn
Copy link
Member

Simn commented Apr 7, 2016

Thanks!

Looks like you're shaping up to be the next Haxe/PHP maintainer!

@mockey
Copy link
Contributor Author

mockey commented Apr 7, 2016

@Simn
Oh great, thanks for merging. I need this for a job targeting PHP at the moment.
What about #4973? The added test should actually pass.

Looks like you're shaping up to be the next Haxe/PHP maintainer!

Well, maybe :-)
I'm trying to figure out the the OCaml code. Where can I ask some questions about it?
Like:

  • Is there something like null in OCaml, that I can return when Meta.get Meta.PhpConstants is not found, so that I don't have to call Meta.get and Meta.has?

  • Would it be maybe better to not call Meta.has Meta.PhpGlobal twice but pass it as var to gen_member_access or is that not so important?

    Also:

  • You must have some printing functions somewhere for debugging, right?

  • Are there some good OCaml tutorials dealing with the general syntax?

Another thing:
I'm a bit worried about that "every condition generates a temp var" behaviour. For a function with a couple of if/else I get about 30 temp vars now, that are also not unset (like other temp vars for loops e.g.). What was the exact reason for this again?

@nadako
Copy link
Member

nadako commented Apr 7, 2016

Is there something like null in OCaml, that I can return when Meta.get Meta.PhpConstants is not found, so that I don't have to call Meta.get and Meta.has?

See http://ocaml-lib.sourceforge.net/doc/Option.html, however in this case it's probably easier to just catch Not_found

Are there some good OCaml tutorials dealing with the general syntax?

This is pretty good, I think: https://realworldocaml.org/v1/en/html/index.html

@mockey
Copy link
Contributor Author

mockey commented Apr 7, 2016

@nadako: Cool, thanks a lot.

@Simn
Copy link
Member

Simn commented Apr 7, 2016

Would it be maybe better to not call Meta.has Meta.PhpGlobal twice but pass it as var to gen_member_access or is that not so important?

Doesn't seem very important but wouldn't hurt either.

I'm a bit worried about that "every condition generates a temp var" behaviour. For a function with a couple of if/else I get about 30 temp vars now, that are also not unset (like other temp vars for loops e.g.). What was the exact reason for this again?

I don't remember what exactly the problem was, but I know there was a problem. You can try removing the PHP check here: https://github.com/HaxeFoundation/haxe/blob/development/src/optimization/analyzerTexpr.ml#L379

Maybe we can figure out the exact cases where it causes trouble and only exclude those, or something.

@mockey
Copy link
Contributor Author

mockey commented Apr 7, 2016

Doesn't seem very important but wouldn't hurt either.

Stupid github-question: Can i update this merged PR or do I have to create a new one?

I don't remember what exactly the problem was, but I know there was a problem.

I think it was some special evaaluation order edge case.

You can try removing the PHP check here

Thanks, will try. Maybe the unit tests will show something.

@Simn
Copy link
Member

Simn commented Apr 7, 2016

Stupid github-question: Can i update this merged PR or do I have to create a new one?

It's gonna require a new one, but this change alone isn't worth a PR. Just add it to the next one you send.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants