Do not emit blatantly illformed Core Erlang apply expressions #256

Merged
merged 1 commit into from Mar 4, 2014

Projects

None yet

3 participants

Contributor
nox commented Mar 1, 2014

(fun f/1)() should be compiled to let X = 'f'/1 in apply X () to let the compiler properly generate code that will fail with badarity at runtime.

@UlfNorell

@nox nox Do not emit blatantly illformed Core Erlang apply expressions
(fun f/1)() should be compiled to let X = 'f'/1 in apply X () to let the compiler
properly generate code that will fail with badarity at runtime.

Reported-by: Ulf Norell
1b8ad68
Contributor
bjorng commented Mar 3, 2014

Thanks! Will run it in our daily builds.

Contributor
bjorng commented Mar 4, 2014

Thinking a little more about this one...

Will this solution always work? What if some Core Erlang optimisation pass would again inline the call of the fun? Would it not be a safer solution to handle this case in v3_kernel or v3_codegen?

Just thinking.

Contributor
nox commented Mar 4, 2014

@bjorng Core lint specifically reject such code, in my opinion if you handle that elsewhere you should make it valid. I would rather say that such code should never be emitted.

@psyeugenic psyeugenic merged commit 1b8ad68 into erlang:master Mar 4, 2014
Contributor
bjorng commented Mar 5, 2014

I failed to notice that fun_inline_SUITE.erl failed to compile. That means that the inliner needs to be fixed too (or fixing the problem somewhere else, for instance in v3_kernel).

Contributor
nox commented Mar 5, 2014

Interesting, will you fix it yourself as the suite in the repos fails or can I look at it tonight?

Contributor
bjorng commented Mar 5, 2014

You can look at it tonight. I will probably be busy with other things today.

For now I have added to our daily builds a branch that reverts your commit.

Contributor
nox commented Mar 5, 2014

@bjorng Oh right, I didn't run this one because I just ran ct_run directly on the compiler test dir. It's quite bothersome to have to run otp_build tests etc, couldn't these kind of suites include the other one with some macro trickery and be included in the repository?

Contributor
nox commented Mar 5, 2014

Nevermind, I can just run make -C lib/compiler/test. Will do that now.

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