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

Add a test for utf8 function names #1439

Merged
merged 1 commit into from May 9, 2017
Merged

Conversation

josevalim
Copy link
Contributor

@josevalim josevalim commented May 4, 2017

This PR improves the coverage for UTF8 function names in compile_SUITE. Someone reported on the mailing list the feature no longer works. The test pass as of ffa80a4 but I still need to run it on latest master.

UPDATE: the test also passes on machine after rebuilding on current master.

@@ -727,6 +727,23 @@ utf8_atoms(Config) when is_list(Config) ->
NoUtf8AtomForms = [{attribute,Anno,module,no_utf8_atom}|Forms],
error = compile:forms(NoUtf8AtomForms, [binary, r19]).

utf8_functions(Config) when is_list(Config) ->
Anno = erl_anno:new(1),
Atom = binary_to_atom(<<"こんにちは"/utf8>>, utf8),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not write this atom directly as 'こんにちは' ?

$ LANG=C.UTF-8 ./bin/erl +pc unicode
Erlang/OTP 20 [DEVELOPMENT] [erts-9.0] [source-95e22b1] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false]

Eshell V9.0  (abort with ^G)
1> 'こんにちは' =:= binary_to_atom(<<"こんにちは"/utf8>>, utf8).                 
true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c0b This uses the same template as the test above and at the time the parser did not handle UTF-8 one. It is probably best to not be dependent on the parser on this case still.

@bjorng bjorng self-assigned this May 4, 2017
@bjorng bjorng added enhancement team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI labels May 4, 2017
@bjorng
Copy link
Contributor

bjorng commented May 8, 2017

The test case compile_SUITE:kernel_listing/1 fails.

@bjorng bjorng removed the testing currently being tested, tag is used by OTP internal CI label May 8, 2017
The test found a bug in v3_kernel_pp which was not
taking into account utf8 atoms. The bug has also
been fixed.
@josevalim
Copy link
Contributor Author

@bjorng so it actually found a bug, just not the one I was expecting. :)

The bug has been fixed and kernel_listing also passes now. Thank you.

@bjorng bjorng added the testing currently being tested, tag is used by OTP internal CI label May 8, 2017
@bjorng bjorng merged commit e1c70e4 into erlang:master May 9, 2017
@josevalim josevalim deleted the jv-atu8-function branch May 9, 2017 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants