-
-
Notifications
You must be signed in to change notification settings - Fork 671
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
WIP: Unicode Support #6748
WIP: Unicode Support #6748
Conversation
… make abstracts simple (easier to deal with)
…xe/i18n/Utf8.unit.hx
… call to __hxcpp_string_of_bytes, see HaxeFoundation/hxcpp#669
} else { | ||
//macro eq($e1, $e2); | ||
macro eqAbstract($e1 == $e2, $e1, $e2); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hughsando can you take a look at this, i have to call eq
when the type is Int for cppia, but only with -D nocppiaast
. if i don't do that it leads to a segfault, but it's only in this special circumstance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nocppiaast is for "legacy test mode" and I don't really want to fix bugs in this code.
Basically, you should not need to use nocppiaast on new code.
Is is acceptable to use this define to hack out the test, or use a "legacy eq" method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can leave it as it is now, but it's totally strange why this happens at all. I mean it just compares integers, i also traced the values, nothing special but at some point, after the first ~90 comparisons it just segfaults.
You can dump out the code with -debug and -D annotate_source and might get
an idea of what the types are doing. It sounds like it is passing an int
as an object or the other way around and getting confused. It is also
possible it is to do with a 1-byte bool vs a 4-byte bool.
…On Thu, Nov 23, 2017 at 7:01 PM, frabbit ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tests/unit/src/unit/UnitBuilder.hx
<#6748 (comment)>:
> + var t2 = Context.follow(Context.typeof(e2));
+ haxe.macro.TypeTools.toString(t1) == haxe.macro.TypeTools.toString(t2);
+ } catch (e:Dynamic) { false; };
+ if (same) {
+
+ #if cppia
+ // calling eqAbstract for Int leads to a segmentation fault with cppia, but only when compiled
+ // with -D nocppiaast
+ var t1 = Context.follow(Context.typeof(e1));
+ var t1 = haxe.macro.TypeTools.toString(t1);
+ if (t1 == "Int") {
+ macro eq($e1, $e2);
+ } else {
+ //macro eq($e1, $e2);
+ macro eqAbstract($e1 == $e2, $e1, $e2);
+ }
I can leave it as it is now, but it's totally strange why this happens at
all. I mean it just compares integers, i also traced the values, nothing
special but at some point, after the first ~90 comparisons it just
segfaults.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6748 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABlp1i6M3f_LeBK_yPQcwkSDm_zu2C3pks5s5VCBgaJpZM4QkZIA>
.
|
@hughsando sounds like fun ;) |
I noticed in this PR that the copyright notice has the wrong years. |
What is the status on this? I see some of the classes in the API, but are they up-to-date? |
Ping, any news here? |
@ncannasse please review |
Yes I had that in a to-review list for too long, sorry about it. Will try to do it soon. |
Ok, so... this one frustrates me a bit because there's so much effort that went into the void. However, at this point I think there's no value in keeping this PR open. The branch is still there in case somebody wants to pick this up and update it to latest Haxe. I could see this becoming a haxelib at first, but this would require a dedicated maintainer. I apologize for how this went down @frabbit. Communication was quite bad from our side, but all we can do now is do better in the future. |
This PR adds support for 4 abstracts, Utf8, Utf16, Ucs2 and Utf32 to the std library. This PR is still work in progress.