[cpp] Lime CFFI issues in 3.4.1 (cannot convert from 'hx::Object *' to 'String') #6103

Closed
Gama11 opened this Issue Mar 17, 2017 · 9 comments

Comments

Projects
None yet
5 participants
@Gama11
Member

Gama11 commented Mar 17, 2017

Regression, bisected to 029cd1e. Isolated from lime.ui.JoyStick (similar errors happen in lime.ui.Gamepad). This breaks 3.4.1 for Flixel (with OpenFL Next) / OpenFL on the cpp target as far as I can tell.

Error: Main.cpp
./src/Main.cpp(50): error C2440: 'return': cannot convert from 'hx::Object *' to 'String'
./src/Main.cpp(50): note: Constructor for class 'String' is declared 'explicit'

class Main {
    static var id:Int;
    static var guid(get,never):String;

    public static function main()
        trace(guid);

    static inline function get_guid()
        return lime_joystick_get_device_guid(id);

    static inline function lime_joystick_get_device_guid(id:Int):Dynamic
        return cffi_lime_joystick_get_device_guid.call(id);

    static var cffi_lime_joystick_get_device_guid:cpp.Callable<StdTypes.Int->cpp.Object> =
        new cpp.Callable<Int->cpp.Object>(cpp.Prime._loadPrime("lime", "lime_joystick_get_device_guid", "io", false));
}

@Simn Simn added the platform-cpp label Mar 17, 2017

@Simn Simn added this to the 3.4 milestone Mar 17, 2017

@Simn Simn added the regression label Mar 17, 2017

@Gama11 Gama11 referenced this issue in openfl/openfl Mar 17, 2017

Closed

Error while building for linux #1531

@jgranick

This comment has been minimized.

Show comment
Hide comment
@jgranick

jgranick Mar 17, 2017

We use :Dynamic as the return type for string values over CFFI. We use a mixture of alloc_string and alloc_wstring when returning strings from CFFI.

I'm not sure if we should using :String as the return type (and if it would work) or use :Dynamic (which is represented as :cpp.Object on the C++ target in our macro, due to issues with plain dynamic)

Perhaps :String is not actually a :cpp.Object?

Anyway, open to suggestions on the right approach if this isn't a bug

We use :Dynamic as the return type for string values over CFFI. We use a mixture of alloc_string and alloc_wstring when returning strings from CFFI.

I'm not sure if we should using :String as the return type (and if it would work) or use :Dynamic (which is represented as :cpp.Object on the C++ target in our macro, due to issues with plain dynamic)

Perhaps :String is not actually a :cpp.Object?

Anyway, open to suggestions on the right approach if this isn't a bug

@hughsando

This comment has been minimized.

Show comment
Hide comment
@hughsando

hughsando Mar 18, 2017

Member

This came when I messed with the typing of TCppReturn to try to work around a issue. If I restore the original:

| TReturn eo ->
            CppReturn(match eo with None -> None | Some e -> Some (retype (cpp_type_of expr.etype) e)), TCppVoid

it works as before, but new stuff breaks.

@Simn I think the real issue is the typing of the "TReturn". Is it the function return type? I guess it would be void otherwise, since you can't do "var x = return ...".
The following code:

class Test
{
   var testVal:Test;
   var testProp(get,null):Test;

   function get_testProp() return testVal;
   public function new()
   {
      trace(testProp);
   }

   public static function main() new Test();
}

Generates the dump:

	function get_testProp[Function:Void -> Test]
		[Block:Dynamic]
			[Return:Dynamic]
				[Field:Test]
					[Const:Test] this
					[FInstance:Test]
						Test
						testVal

Note the typing of Return:Dynamic - but should it be Test ?

Member

hughsando commented Mar 18, 2017

This came when I messed with the typing of TCppReturn to try to work around a issue. If I restore the original:

| TReturn eo ->
            CppReturn(match eo with None -> None | Some e -> Some (retype (cpp_type_of expr.etype) e)), TCppVoid

it works as before, but new stuff breaks.

@Simn I think the real issue is the typing of the "TReturn". Is it the function return type? I guess it would be void otherwise, since you can't do "var x = return ...".
The following code:

class Test
{
   var testVal:Test;
   var testProp(get,null):Test;

   function get_testProp() return testVal;
   public function new()
   {
      trace(testProp);
   }

   public static function main() new Test();
}

Generates the dump:

	function get_testProp[Function:Void -> Test]
		[Block:Dynamic]
			[Return:Dynamic]
				[Field:Test]
					[Const:Test] this
					[FInstance:Test]
						Test
						testVal

Note the typing of Return:Dynamic - but should it be Test ?

@Simn

This comment has been minimized.

Show comment
Hide comment
@Simn

Simn Mar 18, 2017

Member

TReturn has always been typed as Dynamic. You're supposed to look at the value it returns to figure out the return type. I don't remember why it is like that, but it was like that when I found it.

Member

Simn commented Mar 18, 2017

TReturn has always been typed as Dynamic. You're supposed to look at the value it returns to figure out the return type. I don't remember why it is like that, but it was like that when I found it.

@nadako

This comment has been minimized.

Show comment
Hide comment
@nadako

nadako Mar 18, 2017

Member

Isn't that because you can do var a:String = return 5 and it would be weird to have that return typed as Int in a case like this?

Member

nadako commented Mar 18, 2017

Isn't that because you can do var a:String = return 5 and it would be weird to have that return typed as Int in a case like this?

@Simn

This comment has been minimized.

Show comment
Hide comment
@Simn

Simn Mar 18, 2017

Member

Right, I think that's the reason. Looks silly in such simple examples, but it makes sense if you have something like a big value-switch in which some cases return.

Member

Simn commented Mar 18, 2017

Right, I think that's the reason. Looks silly in such simple examples, but it makes sense if you have something like a big value-switch in which some cases return.

@hughsando

This comment has been minimized.

Show comment
Hide comment
@hughsando

hughsando Mar 18, 2017

Member

Ok, I'll have to track "current function return type" if it't not stored anywhere, since this could be different from the type of the eexpr that gets returned (at least, using the typedef hack)

Member

hughsando commented Mar 18, 2017

Ok, I'll have to track "current function return type" if it't not stored anywhere, since this could be different from the type of the eexpr that gets returned (at least, using the typedef hack)

@hughsando

This comment has been minimized.

Show comment
Hide comment
@hughsando

hughsando Mar 18, 2017

Member

Makes me wonder actually if there is something different about the properties, since it seems to work elsewhere (just using the type of the returned expression).

Member

hughsando commented Mar 18, 2017

Makes me wonder actually if there is something different about the properties, since it seems to work elsewhere (just using the type of the returned expression).

@nadako

This comment has been minimized.

Show comment
Hide comment
@nadako

nadako Mar 18, 2017

Member

"current function return type" if it't not stored anywhere

Why wouldn't toplevel TFunction expression's tfunc.tf_type or even field TFun type work here?

Member

nadako commented Mar 18, 2017

"current function return type" if it't not stored anywhere

Why wouldn't toplevel TFunction expression's tfunc.tf_type or even field TFun type work here?

@hughsando

This comment has been minimized.

Show comment
Hide comment
@hughsando

hughsando Mar 18, 2017

Member

Yes - just need to track it to the TReturn expression

Member

hughsando commented Mar 18, 2017

Yes - just need to track it to the TReturn expression

@hughsando hughsando closed this in 433aed4 Mar 18, 2017

@Gama11 Gama11 referenced this issue in openfl/lime Mar 19, 2017

Closed

Joystick.cpp issue on Haxe 3.4.1 #943

markknol added a commit to markknol/haxe that referenced this issue Mar 28, 2017

merge master
* [php7] correctly detect write context to optimize more cases of array access

* and again

* [php7] Remove filehandle import in resource (#5989)

`sys.io.File has no field or subtype FileHandle`

* Fix some spelling errors in docs (#5991)

* [php7] print haxe positions above each line of generated php code

* [php7] do not print Haxe positions if -D real-position is defined

* [lua] add arguments and docs to the Math.random* functions

* [php7] dereference only if really needed

* Do not rewrite path for abstracts when applying @:native (closes #5993)

We don't need to do this, because it's already done for their implementation class as a result of forwarding @:native to them on creation,
and only classes can be restored, not abstracts, as far as I understood.

* mention #5993 in CHANGES

* [display] don't complain about body-less functions in completion (closes #5988)

* [php7] php.NativeStructArray for associatie arrays used as a set of options

* [lua] fix and beautify lua.Math extern

(docs taken from https://www.lua.org/manual/5.1/manual.html#5.6)

* [lua] fix issues with typedef/instance/static assignment fixes #6007

* fix Issue6007 for other platforms

* fix Issue6007 for flash

* [lua] disable fusion on Table.create

* [lua] add test for issue6001

* fix Issue6001

* reverse the reversal (closes #6015)

* [cpp] StringBuf - only flush the charBuf once.  Fixes #6018

* [cpp] Use @:native gc functions instead of __global__.  Fixes #6014

* Fixed missing separator after __properties__ (#6024)

* [js] don't inline Reflect.(get|set)Property methods, as those are too big and messy for that

* [cpp] Mark Gc functions as extern.

* [AppVeyor] fix php installation

* [cpp] Allow host ip of 0, if name is '0.0.0.0' closes #5987

* [cpp] Make Gc extens cppia friendly

* update haxelib (#6032)

* [php] fixed invalid result of Web.getPostData() (fixes #6073)

* typo

* [lua] fix #6037

* [lua] focus type parameter for lua/coroutine

* [php7] Actually withdraw fetched row (#6041)

* [php7] Actually withdraw fetched row

* [php7] a test for #6041

* fix random syntax

* fix some names in unitstd

and hope that these aren't broken

* Revert "fix some names in unitstd"

This reverts commit 466aa3c.

* [lua] avoid allocation with type.getEnumParameters

* [php7] throw EOF if reading from BytesBuffer reached the end (fixes #6043)

* [php7] Save stack of a caught exception only if CallStack.exceptionStack() is used somewhere (fixes #6046)

* [php7] dont print positions for blocks in generated code

* [lua] handle luaRequire import declarations in the type_forward generator

* [lua] use local inline function for Type.getEnumParameters

* [lua] use unsafe cast in Type.enumParameters

* [lua] do some bounds checking for Std.int, and return 0 if infinite/nan

* [lua] double-escape lua Ereg group match capture tokens

* support toString on Any, so it behaves similar to Dynamic when used in string concatenation/interpolation

* [analyzer] do not bind `@:structAccess` to temp vars

* bugfix OpAssignOp on some arrays

* fix correct file close

* make generated `return` position match the position of an expr being returned from a complex expr

this should help js source maps and other debugging

* Sourcemap support for php7 (#6052)

* sourcemap generator draft

* trying to update Makefile for sourcemaps

* fix sourcemap_file#new_line

* added sourcemap generator to genphp7 as proof-of-concept

* sourcemap_builder draft

* [php7] refactor write_args

* [php7] use parent_expr instead of expr_hierarchy for cleaner code

* [php7] all output through type_builder#write

* fix line numbers in sourcemap

* fix accumulating sourcemaps of all previousely generated files

* fix line mapping if stack trace does not provide coumn information

* [php7] correctly build sourcemap for closures

* fix typo

* source_map define

* [php7] haxe.CallStack.mapPosition for pretty call stacks

* [lua] fix Ffi typing problems

* [php7] use strlen for strings in FileInput.readBytes (#6055)

* use strlen for strings in FileInput.readBytes

* fixed unitest to close test file

* [php7] remove redundant case in field access generation

* call db.close instead of this.close to avoid recursion (#6056)

* changelog

* [php] test for #6057

* [php7] fix String.indexOf() default values for `startIndex` argument

* Fix for #6057 FileSystem.stat() fails on directories (#6058)

Looks like it's a windows-only issue. Thanks for the fix.

* changelog

* remove wild spaces

* write errors to stdout instead of stderr because our output goes to stderr instead of stdout

Don't ask me

* use --php-front parameter (#6060)

* [php] setup appveyor

* fix #6067

* added gc flags and tracking api

* update hl.Gc.track

* added obj as callb param

* add test for #6067, mention the fix in changelog

* [php7] maintain statics initialization order (fixes #6070)

* [display] check whether the type we want to get signature for is actually callable (closes #6068)

* [display] store signatures directly in DisplaySignatures, instead of TFun - less run-time checks, more type safety \o/

* Do not generate scriptable glue for callable variables. Closes HaxeFoundation/hxcpp#573

* bump version

* [cpp] Fix is_data_member call

* silently create unbound var infos instead of asserting

* [cpp] Return CallableData instead of T for 'call' member

* do not lose field type when calling a var/property function

* minor

* [php7] generator refactoring

* [php7] support haxe.EntryPoint

* add a function to print out current unification stacks

* [cpp] Make cpp.Funciton.call return FunctionData

* fix structural extension display position

closes #6029
closes #6077

* [php7] dreference field access on `null`; more cases for #4988

* [php] fix null.nonExistent()

* disable test for `null.nonExistent()` on lua and as3

* disable test for `null.nonExistent()` on lua and as3

* only account actual typing for the "macro typing" timer

* Update Array.hx (#6084)

minor addition to clarify result is return value

* [php] changed _hx_cast() to use _hx_instancof() instead of Std.is() to avoid handling php-prefix (fixes #6083)

* [lua] put static initialization in helper method and delay evaluation.  fix #6087

* update submodule

* CHANGES.txt update

* [lua] fix missing type handler

* [cpp] Allow raw native pointers (cpp.Star) and structs in hxcpp, and explicitly add the dynamic conversions where required

* added sentinel pause

* [cpp] Add line number entry at start of functions to allow breaking on entering.

* [lua] use custom Eof with local dce disabled

* [lua] better error messages for missing/incorrect file input

* Revert "[lua] use custom Eof with local dce disabled"

This reverts commit bd526d5.

* [lua] avoid duplicate init calls, the check features routine is no longer necessary

* update release procedure
[skip ci]

* [lua] Lua.setmetatable returns a table

* [lua] drop unnecessary Map class, add some helpers to PairTools

* [lua] nah, pairtool methods probably belong in a 3rd party lib

* UNC paths are absolute paths as well (#6061)

* Fix fail to inline extern constructor not producing an error #6034 (#6035)

* Fix inline extern constructors failing to inline not producing an error #6034

* inline constructors bugix optimization:
Keep count of extern inline constructors and only do the third pass if the count is not 0.

* No third pass is needed, check in the second pass if there are extern inline constructors left.

* Manually fix broken API docs of HTMLDocument http://api.haxe.org/js/html/HTMLDocument.html (#5936)

I provided a fix for future here HaxeFoundation/html-externs#7

* java.Lib: remove confusing deprecated docs (#6026)

Recursion. See also: recursion.

* Fix inline constructor field initialization for super constructors. #6093 (#6097)

* Handle inline constructor field initialization for super constructors too. Fixes #6093

* Reuse cl_params map.

* Add Issue6093 unit test.

* Add Issue6093 optimization test.

* Fix EnumFlags.unset() (#6019)

* CHANGES.txt update [skip ci]

* fix date

* Date - Minor API documentation copy fix

Remove "the".

* it's 2017

* Fix typo in FileSystem.isDirectory() docs (#6104)

* [tests] don't assume test methods with optional args will work when called throught Reflect.callMethod with no arguments - we don't specify that.

* [cpp] Do not qualify @:native types with global namespace.  Closes #6086

* OCD [skip ci]

* remove unused version_is_stable value [skip ci]

* [cpp] Store the expected return type for a function. Closes #6103

* [php7] wrap empty array declaration in parentheses if used directly with array access (fixes #6090)

* [cpp] Don't emit a line change on the first line of a function

* Improved doc wording for Sys.environment() (#6109)

* prepare 3.4.2

* label loops with breaks within switch with a number (algorhithm from gencommon)

* [js] generate labels for loops with breaks within switch (closes #3883)

* [js] don't generate unneeded switch breaks

* mention #4964 in CHANGES.txt [skip ci]

* only keep haxe.io.Eof.toString if Eof was actually used

* spaces->tabs

* `callback` is long gone

* get rid of silly argument name shortening in .bind()

* [display] show fn.bind as a method in field completion (closes #6004)

* added Lib.print and Lib.println (fixed #5593) (#5594)

* added Lib.print and Lib.println (fixed #5593)

* inline function and replace untyped

* remove untyped java

* update getLocalStorage/getSessionStorage check (#6079)

Closes #6050

If you are browsing private on iOS, localstorage is available but you cannot write in it. It'll throw "QUOTA_EXCEEDED_ERR: DOM Exception 22: An attempt was made to add something to storage that exceeded the quota."

This can be solved in the detection of localstorage by trying to write something in storage

* [cs/java] adjust tests after 9203fbb

* [cs] don't generate `string.Equals`  for string equality checks - a == b looks better and behaves exactly the same

* remove obsolete comment [skip ci]

* remove some dead code [skip ci]

* [cs/java] remove IteratorsInterfaceExprf, because `TFor`s are always rewritten to while in the analyzer

* [gencommon] remove old comments [skip ci]

* [gencommon] remove commented out IteratorsInterface. tried it - doesn't really work well with other filters. better look into possibility to generate anon structure interfaces in general

* get rid of SPOD dependency for #5757 test

* [lua] streamline Std.string(Std.string(...)) declarations to Std.string(...)

* [lua] avoid using dynamic/untyped for table creation

* [lua] OCD indent/spacing tweaks

* remove SPOD (#6115)

* remove SPOD

* mention SPOD removal in CHANGES.txt [skip ci]

* [js] remove deprecated JQuery and SWFObject externs (#6116)

* try installing curl from chocolatey

* update haxelib [skip ci]

* [cs] rework enum-to-class generation (#6119)

* [cs] rework enum-to-class generation

* fix generated enum classes priority

* add latest stuff to CHANGES.txt

* [cpp] Fix return typing for embedded closures. Closes #6121

* move toplevel files to src/compiler

Also make Globals not depend on Version

* rename parser.ml to parser.mly

Technically it's not a mly file, but tool automation is annoying if there's a single file that requires the camlp4 preprocessor.

* add Makefile/sh based "build system"

* Some travis cleanup (#6110)

* [travis] use neko PPA instead of building it

* install OCaml through opam

* run Php7 tests as part of linux tests

* actually run php7 tests

* quotes?

* disable sauce connect until we figure out how to avoid forks breaking

* split gencommon, moving its actual filters into separate modules (#6122)

* fix a type inference warning

* adjust .merlin and finally fix that genlua warning

* move .NET type loading into a separate module, so we don't depend on Gencs for that

* Dotnet module actually belongs to typing, not generation

* move Java type loading into a separate module, so we don't depend on genjava->gencommon for everything

* move SWF type loading into a separate module so we don't depend on SWF generator for that

* spaces -> tabs [skip ci]

* random gencommon cleanup

* random gencommon cleanup 2

* [gencommon] make mk_temp and mk_internal_name simple functions, not tied to gencommon context

* [gencommon] remove unused events

* [gencommon] fix double return in reflection helper exprs (also make mk_return type the TReturn as t_dynamic, like we do in other places)

* [gencommon] move mk_return to Codegen and use it instead of manually constructing TReturn Some

* [gencommon] remove some code duplication

* [gencommon] get rid of Gencommon dependency for SetHXGen filter

* simplify rule_map_dispatcher#add - make its arguments non-optional

* simplify rule_dispatcher a bit

* minor

* move gencs/genjava's configure function inside generate, because it actually generates stuff, not merely configures

* [gencommon] extract RenameTypeParameters into a separate module (also get rid of gencommon context dependency, as it only needs types)

* [gencommon] split up TypeParams module

it really is just the "RealTypeParams" filter and the infer_params function that are used by other methods and don't even need gencommon,
so move infer_params into gencommon (for now), and move the rest into RealTypeParams module

* fix with latest hl.h

* [gencommon] move infer_params into CastDetect.handle_type_parameter as it's the only place where it's used

* [gencommon] cleanup ClassInstance a bit (also cache the __typeof__ var instead of re-creating it for each access)

* sync with extlib changes

* [gencommon] cleanup DefaultArguments, get rid of some code duplication

* [gencommon] cleanup DynamicFieldAccess

* [gencommon] clean up DynamicOperators (also make the actual filter only depend on common context)

* [gencommon] cleanup SwitchBreakSynf (also remove its gencommon context dependency, add a TODO that it's to be removed)

* [gencommon] get rid of gencommon context dependency for UnnecessaryCastsRemoval and UnreachableCodeEliminationSynf filters

* [gencommon] minor [skip ci]

* [gencommon] cleanup IntDivisionSynf and get rid of gencommon context for the actual filter

* [gencommon] remove InitFunction dependency on gencommon context

* [gencommon] cleanup ExpressionUnwrap a bit

* [gencommon] cleanup and decouple from gencommon context InterfaceProps and Normalize (also fix normalize checking strict_meta instead of metadata_entry)

* [gencommon] use info from Filters.mark_switch_break_loops and remove SwitchBreakSynf

* Replace abstracts names with native name if specified

* use ocamlfind (#6123)

* [gencommon] clean up TryCatchWrapper and lose gencommon context dependency in the actual filter code

* add a comment [skip ci]

* [gencommon] cleanup AbstractImplementationFix

* [gencommon] clean up ArrayDeclSynf, lose gencommon context dependency

* [gencommon] clean up ClassInstance, lose gencommon context dependency (also the gspecial_vars check, but it doesn't seem to be needed)

* [gencommon] minor

* [gencommon] get rid of redundant Option wrapping for filters results (if we don't change expr/type, then we might as well just return it as is)

+ a couple of very minor changes

* [gencommon] minor

* [gencommon] cleanup cast method generation a bit

* [gencommon] minor

* [CI] fixed haxe_ver parsing

* [Makefile] keep the CFLAGS declaration

Such that make wouldn't pickup any CFLAGS env var, which happens to be defined in launchpad.
We probably shouldn't even use CFLAGS, since our source is not C... Maybe MLFLAGS or something would be better.

* [CI] clean up

* [CI] ubuntu wily is not supported anymore

* [cs/java] use native arrays for dynamic function invokation (#6125)

* [gencommon] minor

* [gencommon] minor (lose gencommon ctx dependency from the ensure_local function)

* don't duplicate field hashing function

* [gencommon] generate nicer ternary for the exception "catchall" unwrapping

* [gencommon] clean up TryCatchWrapper configuration a bit

* minor [skip ci]

* make dec/enc/decode/encode naming in macro interpreter consistent

* have a generic make_throw in ExprBuilder

* use ExprBuilder some more

* [cs/java] throw target native argument exceptions in closure classes

* [cs/java] throw nice-ish target-specific exceptions for bad dynamic field access

* [cs/java] run TryCatchWrapper as a normal filter before analyzer and DCE - much cleaner output \o/ (also lose any gencommon dependency) (#6128)

* small cleanup for TryCatchWrapper

* [cs/java] make static constructrors protected

* [gencommon] lose gencommon context dependency for the most OverloadingConstructor filter code

* [gencommon] lose gencommon context dependency for DefaultArguments

* [gencommon] reset temp vars before creating a type param cast method

* [AppVeyor] use the new host of fdopen's opam32.tar.xz

fdopen/opam-repository-mingw#26

* [AppVeyor] make curl support redirect

* [cs/java] extract DefaultArguments from gencommon into PASS 1.5 filters (#6134)

* run SetHXGen in filters

* [cs/java] move DefaultArguments filter into pre-analyzer/dce stage

* get rid of gencommon dependency for the DefaultArguments filter

* [cs/java] remove InterfaceMetas filter since `Filters.add_meta_field` does exactly the same

* minor cleanup for Filters.add_meta_field: only create field once, add _HxMeta type to the end of the types list (it matters when generating multiple types per module like it's done in C#)

* fix minor typo in Type.getEnumConstructs [skip ci]

* [cs/java] carry over @:protected meta when copying constructor

* [cs] optimize Type.getEnumConstructs and mark some enum class fields as protected

* Don't require Bool to be an enum (closes #5804, closes #4120) (#6137)

* [cs] some cleanup and optimizations for Type methods

* [cs] more random Type optimizations

* [gencommon] cleanup InitFunction a bit

* [cs] only generate __hx_constructs when Type.getEnumConstructs is there

* [python] use native startswith/endswith in StringTools (closes #6138)

* some cleanup for EnumToClass2 in preparation for java support (and extracting from gencommon)

* [gencommon] reuse Type.mk_field in Gencommon.mk_class_field

* [cs] move InterfaceProps into post-dce type filters, lose any gencommon dependency

* added hl libuv support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment