Skip to content

Commit

Permalink
Merge pull request #230 from gomoripeti/otp_26
Browse files Browse the repository at this point in the history
Fixes to support OTP 24..26 and Elixir 1.13..1.15
  • Loading branch information
gomoripeti authored Jan 11, 2024
2 parents f17d584 + 98911bd commit fb26112
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 41 deletions.
61 changes: 51 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ jobs:
fail-fast: false
matrix:
include:
- otp_version: 26
# hdr_histogram NIF requires the below PR to be merged to build on OTP 26
# https://github.com/HdrHistogram/hdr_histogram_erl/pull/44
erl_hist: true

- otp_version: 25

- otp_version: 24

- otp_version: 23

- otp_version: 22
Expand Down Expand Up @@ -117,51 +126,83 @@ jobs:

test_elixir:
name: Test with Elixir (Elixir ${{matrix.elixir}} | OTP ${{matrix.otp_version}})
runs-on: ubuntu-18.04
runs-on: ${{matrix.os}}
strategy:
fail-fast: false
matrix:
include:
- elixir: 1.4.x
otp_version: 18
rebar3: '3.7.4'
old_cowboy: true
# with hdr_histogram NIF: xprof_core_hist_SUITE: Segmentation fault (core dumped)
erl_hist: true
# runner ubuntu-20.4 is only compatible with OTP 20+
# earlier OTP versions fail with:
# no match of right hand side value: {:error, {:ssl, {'no such file or directory', 'ssl.app'}}}
# running ubuntu-18.4 was removed on 2022.04.03
# https://github.com/actions/runner-images/issues/6002
# - elixir: 1.4.x
# otp_version: 18
# rebar3: '3.7.4'
# old_cowboy: true
# # with hdr_histogram NIF: xprof_core_hist_SUITE: Segmentation fault (core dumped)
# erl_hist: true
# os: ubuntu-18.04

- elixir: 1.5.x
otp_version: 18
otp_version: 20
rebar3: '3.7.4'
old_cowboy: true
erl_hist: true
os: ubuntu-20.04

- elixir: 1.6.x
otp_version: 19
otp_version: 20
rebar3: '3.15.2'
os: ubuntu-20.04

- elixir: 1.7.x
otp_version: 19
otp_version: 20
rebar3: '3.15.2'
os: ubuntu-20.04

- elixir: 1.8.x
otp_version: 20
rebar3: '3.15.2'
os: ubuntu-20.04

- elixir: 1.9.x
otp_version: 20
rebar3: '3.15.2'
os: ubuntu-20.04

- elixir: 1.10.x
otp_version: 21
rebar3: '3.15.2'
os: ubuntu-20.04

- elixir: 1.11.x
otp_version: 22
rebar3: '3.18.0'
os: ubuntu-20.04

- elixir: 1.12.x
otp_version: 23
rebar3: '3.19.0'
os: ubuntu-20.04

- elixir: 1.13.x
otp_version: 24
rebar3: '3.20.0'
os: ubuntu-latest

- elixir: 1.14.x
otp_version: 25
rebar3: '3.21.0'
os: ubuntu-latest

- elixir: 1.15.x
otp_version: 26
rebar3: '3.22.1'
# hdr_histogram NIF requires the below PR to be merged to build on OTP 26
# https://github.com/HdrHistogram/hdr_histogram_erl/pull/44
erl_hist: true
os: ubuntu-latest

steps:
- uses: actions/checkout@v3
Expand Down
70 changes: 61 additions & 9 deletions apps/xprof_core/src/xprof_core_elixir_syntax.erl
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,15 @@ tokenizer(Str, StartColumn) ->
unify_tokenizer_output(
elixir_tokenizer:tokenize(Str, 1, StartColumn, [])).

unify_tokenizer_output({ok, _Line, _Column, _Warnings, Tokens}) ->
{ok, Tokens};
unify_tokenizer_output({ok, Tokens}) ->
{ok, Tokens};
unify_tokenizer_output({ok, _Line, _Column, Tokens}) ->
%% FIXME old format returned before Elixir 1.6.0
{ok, Tokens};
unify_tokenizer_output({error, {_Line, _Column, Error, TokenHint}, Rest, _Warnings, SoFar}) ->
{error, {Error, TokenHint, Rest, SoFar}};
unify_tokenizer_output({error, {_Line, _Column, Error, TokenHint}, Rest, SoFar}) ->
{error, {Error, TokenHint, Rest, SoFar}};
unify_tokenizer_output({error, {_Line, Error, TokenHint}, Rest, SoFar}) ->
Expand Down Expand Up @@ -434,24 +438,72 @@ mod_to_atom({__aliasis, _, _} = Quoted) ->
%% @doc Convert an Elixir quoted expression to an Erlang abstract syntax tree
-spec quoted_to_ast(ex_quoted()) -> {ok, erl_ast()} | {error, Reason :: string()}.
quoted_to_ast(Quoted) ->
case erlang:function_exported('Elixir.Code', with_diagnostics, 1) of
true ->
%% Since Elixir 1.15.0 the compiler can emit a list of
%% warnings and errors, but does not return them
case 'Elixir.Code':with_diagnostics(
fun() -> quoted_to_erl(Quoted) end) of
{{ok, Ast}, _} ->
{ok, Ast};
{{error, _}, Diagnostics} ->
fmt_elixir_diagnostic(Diagnostics)
end;
false ->
case quoted_to_erl(Quoted) of
{ok, Ast} ->
{ok, Ast};
{error, Exception} ->
case 'Elixir.Exception':'exception?'(Exception) of
true ->
fmt_elixir_exception(Exception);
false ->
erlang:error(Exception)
%%xprof_core_lib:err("cannot convert quoted expression to Erlang AST")
end
end
end.

quoted_to_erl(Quoted) ->
%% pretend that the quoted expression is within a function,
%% this way local function calls are allowed
%% (also expressions which look like local function calls
%% like 'return_trace()' required for fun2ms)
Env = maps:put(function, {ms, 0}, elixir:env_for_eval([])),

try elixir:quoted_to_erl(Quoted, Env) of
{Ast, _NewEnv, _Scope} -> {ok, Ast}
{Ast, _NewEnv, _Scope} ->
%% before Elixir 1.13.0 the return value of
%% `elixir:quoted_to_erl/2` had a different format
{ok, Ast};
{Ast, _NewErlS, _NewExS, _NewEnv} ->
{ok, Ast}
catch error:Exception ->
case 'Elixir.Exception':'exception?'(Exception) of
true ->
xprof_core_lib:fmt_err(
"~ts", ['Elixir.Exception':message(Exception)]);
false ->
erlang:error(Exception)
%%xprof_core_lib:err("cannot convert quoted expression to Erlang AST")
end
{error, Exception}
end.

fmt_elixir_diagnostic(Diagnostics) ->
%% Ignore warnings and take the message from the first error
[#{message := Msg}|_] = [D || D = #{severity := error} <- Diagnostics],
xprof_core_lib:fmt_err("~ts", [Msg]).

fmt_elixir_exception(Exception) ->
Message =
%% remove file+line prefix
case 'Elixir.Exception':message(Exception) of
<<"nofile:1: ", Rest/binary>> ->
Rest;
<<"nofile:1:", Rest/binary>> ->
%% keep column number
Rest;
<<"nofile: ", Rest/binary>> ->
%% no line number - unlikely
Rest;
Msg ->
Msg
end,
xprof_core_lib:fmt_err("~ts", [Message]).

%% @doc Convert an Elixir-style camel-case alias to an Erlang-style snake-case
%% atom.
alias_to_cmd_atom(CmdAlias) when is_atom(CmdAlias) ->
Expand Down
10 changes: 8 additions & 2 deletions apps/xprof_core/src/xprof_core_ms.erl
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,14 @@ ms(Clauses, RecDefs) ->
%% (see https://github.com/erlang/otp/commit/8db6c68b)
workaround_empty_args_ms(
ms(workaround_empty_args_cl(Clauses), RecDefs));
{error,[{_, [{Loc ,ms_transform, ERR_HEADMATCH}]}], _} ->
xprof_core_lib:err(Loc, ?MODULE, ms_transform_headmatch);
{error,[{_, [{_Loc ,ms_transform, ERR_HEADMATCH}]}], _} ->
%% Before OTP 24.0 the column of a match expression was
%% the column of the `=' sign. Since OTP 24.0 erl_parse
%% sets it to the start column of the left-hand-side
%% expression. This later value does not give a nice
%% looking, additional info so let's not include column in
%% the error
xprof_core_lib:err(1, ?MODULE, ms_transform_headmatch);
{error,[{_,[{Loc,Mod,Code}|_]}|_],_} ->
xprof_core_lib:err(Loc, Mod, Code);
MS ->
Expand Down
33 changes: 18 additions & 15 deletions apps/xprof_core/test/xprof_core_elixir_syntax_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
-define(M, xprof_core_elixir_syntax).

parse_match_spec_test_() ->
%% some literals loose their location information in elixir_parser
%% but it is backfilled later by string_to_quoted
%% LiteralLine is 0 until Elixir 1.12, and 1 since 1.13
Tests =
[%% arity explicitly defined
?_assertEqual({mfa,{'Elixir.Mod','fun',1}},
Expand All @@ -29,13 +32,13 @@ parse_match_spec_test_() ->
[],
[{call,1,{atom,1,message},[{var, 1, Var_a}]}]}]},
?M:parse_match_spec("App.Mod.fun a, _ -> message a")),
?_assertEqual({clauses,mod,'fun',
[{clause,1,[{atom,0,ok}],[],[{atom,0,true}]}]},
?_assertMatch({clauses,mod,'fun',
[{clause,1,[{atom,LiteralLine,ok}],[],[{atom,LiteralLine,true}]}]},
?M:parse_match_spec(":mod.fun(:ok) -> true")),
?_assertMatch({clauses,'Elixir.Mod','fun',
[{clause,1,
[{var,1,Var_a}],
[[{op,1,'>',{var,1,Var_a},{integer,0,1}}]],
[[{op,1,'>',{var,1,Var_a},{integer,_LiteralLine,1}}]],
[{call,1,{atom,1,return_trace},[]}]}]},
?M:parse_match_spec("Mod.fun(a) when a > 1 -> return_trace()")),

Expand Down Expand Up @@ -70,10 +73,10 @@ parse_match_spec_test_() ->
catch ?M:parse_match_spec("a+b ->")),
%% fn_to_clauses
?_assertMatch(
{error,"nofile:1: cannot invoke remote function Mod.fun/1 inside " ++ _},
{error,"cannot invoke remote function Mod.fun/1 inside " ++ _},
catch ?M:parse_match_spec("Mod.fun(1) -> true; Mod.fun(2) -> false")),
?_assertMatch(
{error,"nofile:1: cannot mix clauses with different arities in" ++ _},
{error,"cannot mix clauses with different arities in" ++ _},
catch ?M:parse_match_spec("Mod.fun(1) -> true; (1, 2) -> false")),

?_assertMatch(
Expand All @@ -85,33 +88,33 @@ parse_match_spec_test_() ->
%% but will fail conversion to matchspec
{clauses,'Elixir.Mod','fun', [_]} -> ok
catch
{error,"nofile:1: cannot invoke remote function :erlang.//2 inside " ++ _} ->
{error,"cannot invoke remote function :erlang.//2 inside " ++ _} ->
%% error message slightly changed in Elixir 1.8.0
ok
end),

%% Valid quoted syntax - shortcuts

%% Missing args and body
?_assertEqual({clauses,'Elixir.App.Mod','fun',[{clause,1,[],[],[{atom,0,true}]}]},
?_assertMatch({clauses,'Elixir.App.Mod','fun',[{clause,1,[],[],[{atom,_LiteralLine,true}]}]},
?M:parse_match_spec("App.Mod.fun")),
?_assertEqual({clauses,mod,'fun',[{clause,1,[],[],[{atom,0,true}]}]},
?_assertMatch({clauses,mod,'fun',[{clause,1,[],[],[{atom,_LiteralLine,true}]}]},
?M:parse_match_spec(":mod.fun")),

%% Only args present, no body
?_assertEqual({clauses,'Elixir.App.Mod','fun',
[{clause,1,[{atom,0,ok}],[],[{atom,0,true}]}]},
?_assertMatch({clauses,'Elixir.App.Mod','fun',
[{clause,1,[{atom,LiteralLine,ok}],[],[{atom,LiteralLine,true}]}]},
?M:parse_match_spec("App.Mod.fun(:ok)")),
?_assertEqual({clauses,mod,'fun',
[{clause,1,[{atom,0,ok}],[],[{atom,0,true}]}]},
?_assertMatch({clauses,mod,'fun',
[{clause,1,[{atom,LiteralLine,ok}],[],[{atom,LiteralLine,true}]}]},
?M:parse_match_spec(":mod.fun(:ok)")),

%% Args and guards present, but no body
?_assertMatch({clauses,'Elixir.App.Mod','fun',
[{clause,1,[{var,1,Var_a}],
[[{op,1,'<',{var,1,Var_a},{integer,0,1}}],
[{op,1,'>',{var,1,Var_a},{integer,0,10}}]],
[{atom,0,true}]}]},
[[{op,1,'<',{var,1,Var_a},{integer,LiteralLine,1}}],
[{op,1,'>',{var,1,Var_a},{integer,LiteralLine,10}}]],
[{atom,LiteralLine,true}]}]},
?M:parse_match_spec("App.Mod.fun(a) when a < 1 when a > 10"))
],
xprof_core_test_lib:run_elixir_unit_tests(Tests).
Expand Down
3 changes: 1 addition & 2 deletions apps/xprof_core/test/xprof_core_ms_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ ensure_body_test_() ->
ms_test_() ->
[?_assertEqual(
{error,
"matching (=) in fun head cannot be translated into a match-spec "
"at column 7"},
"matching (=) in fun head cannot be translated into a match-spec"},
?M:fun2ms("m:f(A = {B, _}) -> {A, B}")),
?_assertEqual(
{error,
Expand Down
6 changes: 3 additions & 3 deletions apps/xprof_core/test/xprof_core_vm_info_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,15 @@ get_called_funs_test_() ->
end},
{"Lists calls from preloaded modules in OTP",
fun() ->
?assertEqual([{erlang, fun_info_1, 3}], ?M:get_called_funs({erlang, fun_info, 1}))
?assertEqual([{auth, get_cookie, 0}], ?M:get_called_funs({erlang, get_cookie, 0}))
end},
{"Doesn't list anything for BIF functions (containing erlang:nif_error/1 call)",
fun() ->
?assertEqual([], ?M:get_called_funs({lists, reverse, 2}))
end},
{"Doesn't list itself in case of recursive function",
fun() ->
?assertEqual([], ?M:get_called_funs({lists, nth, 2}))
?assertEqual([], ?M:get_called_funs({lists, prefix, 2}))
end},
{"Extract calls from loaded xprof modules",
fun() ->
Expand Down Expand Up @@ -198,7 +198,7 @@ get_available_funs_elixir_test_() ->
%% let's expand Task.Supervisor
L2 = ?M:get_available_funs(<<"Task.">>),
?assert(lists:member(<<"Task.start_link/1">>, L2)), %% exported
?assert(lists:member(<<"Task.exit/2">>, L2)), %% local
?assert(lists:member(<<"Task.flush_reply/1">>, L2)), %% local

?assertNot(erlang:module_loaded('Elixir.Calendar')),
?assertNot(erlang:module_loaded('Elixir.Calendar.ISO'))
Expand Down

0 comments on commit fb26112

Please sign in to comment.