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

tf_args expr #7439

Merged
merged 13 commits into from Oct 10, 2018

Conversation

Projects
None yet
4 participants
@Simn
Copy link
Member

Simn commented Sep 18, 2018

From #4287.

  • hl
  • as3
  • cpp

@Simn Simn added this to the Release 4.0 milestone Sep 18, 2018

@skial skial referenced this pull request Sep 19, 2018

Closed

Haxe Roundup 448 #542

1 of 1 task complete
@Simn

This comment has been minimized.

Copy link
Member

Simn commented Sep 27, 2018

As3 done!

@hughsando

This comment has been minimized.

Copy link
Member

hughsando commented Oct 1, 2018

@Simn, I have no idea how to use git to get the changes to you - please try the attaches diff.
hugh.zip

Simn added some commits Oct 1, 2018

@Simn

This comment has been minimized.

Copy link
Member

Simn commented Oct 1, 2018

Travis says that there's a problem with native compilation:

 - src/unit/issues/_Issue5340/Vector.cpp 
Error: In file included from ./src/unit/issues/Issue5340.cpp:17:0:
include/unit/issues/_Issue5340/Vector.h: In static member function ‘static hx::ObjectPtr<unit::issues::_Issue5340::Vector_obj> unit::issues::_Issue5340::Vector_obj::__alloc(hx::Ctx*, Dynamic)’:
include/unit/issues/_Issue5340/Vector.h:43:71: error: operands to ?: have different types ‘Dynamic’ and ‘double’
                ::Dynamic x = hx::IsNotNull(__o_x) ? __o_x : ((Float)0.);

I cannot seem to reproduce that locally though.

@hughsando

This comment has been minimized.

Copy link
Member

hughsando commented Oct 1, 2018

Looks like a gcc v msvc thing. I usually compile with android on windows to reproduce these.

@hughsando

This comment has been minimized.

Copy link
Member

hughsando commented Oct 10, 2018

This seems to work on linux.
gencpp.zip

@Simn

This comment has been minimized.

Copy link
Member

Simn commented Oct 10, 2018

Thanks, applied!

@Simn

This comment has been minimized.

Copy link
Member

Simn commented Oct 10, 2018

Cpp confirmed passing! Only HL left.

@ncannasse

This comment has been minimized.

Copy link
Member

ncannasse commented Oct 10, 2018

I fetched your branch but can't commit to it.
Simply replace the assert false by op ctx (OMov (r, eval_to ctx c vt)); (tested , it passes the unit tests)

@Simn

This comment has been minimized.

Copy link
Member

Simn commented Oct 10, 2018

Done, thanks. I wonder why GitHub doesn't let you guys push to this branch. That usually works fine when opening a PR...

@Simn

This comment has been minimized.

Copy link
Member

Simn commented Oct 10, 2018

The tests pass. I'll go ahead and merge now, but we still have to discuss if we want to allow Some(1) as default value. I'll reopen #4287 for that.

@Simn Simn merged commit c76e7b3 into HaxeFoundation:development Oct 10, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@Simn Simn deleted the Simn:tf_args_expr branch Oct 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment