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

Fix [@@inline] with default parameters in flambda #880

Merged
merged 2 commits into from Nov 3, 2016

Conversation

Projects
None yet
4 participants
@lpw25
Copy link
Contributor

commented Oct 28, 2016

Currently, functions with default parameters that are marked [@@inline] can cause a compilation error with flamda:

let foo ?(bar=`Default) () =
  Array.make 5 bar
[@@inline]

let _ = foo ()

gives:

>> Fatal error: Stubs may not be annotated as [Always_inline] or [Unroll]: (let
                                                             (bar/18
                                                                *(let
                                                                   (cond/19
                                                                    **opt*/17)
                                                                   (if
                                                                    cond/19
                                                                    then begin
                                                                    (let
                                                                    (Pfield/21
                                                                    (field 0<{test.ml:2,14-22}>
                                                                    *opt*/17))
                                                                    Pfield/21)
                                                                    end else begin
                                                                    (let
                                                                    (const_pointer/20
                                                                    Const(-384499551a))
                                                                    const_pointer/20)
                                                                    end))
                                                              apply_funct/22
                                                                *foo_inner/15)
                                                             (apply<>
                                                               apply_funct/22
                                                               bar/18
                                                               param/16))

Fatal error: exception Misc.Fatal_error

This is because the [@@inline] attribute is copied onto the wrapper function for the default parameter. This patch prevents the attribute from being copied in flambda.

Since it is a bug fix, I've made the PR against 4.04, but since it is a compilation error rather than a miscompilation it can safely be cherry-picked onto trunk instead if people prefer.

@mshinwell

This comment has been minimized.

Copy link
Contributor

commented Oct 28, 2016

This looks ok.

@damiendoligez Can we get this in 4.04? (This has actually appeared when people have been using flambda.)

@mshinwell

This comment has been minimized.

Copy link
Contributor

commented Oct 28, 2016

@lpw25 This should have a Changes entry though I think

@lpw25

This comment has been minimized.

Copy link
Contributor Author

commented Oct 28, 2016

This should have a Changes entry

Done

@chambart

This comment has been minimized.

Copy link
Contributor

commented Oct 28, 2016

That seems like the right fix.

@damiendoligez damiendoligez added this to the 4.04 milestone Nov 2, 2016

@damiendoligez damiendoligez added the bug label Nov 2, 2016

@damiendoligez damiendoligez merged commit d2ff741 into ocaml:4.04 Nov 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

lpw25 added a commit to janestreet/ocaml that referenced this pull request Nov 4, 2016

dra27 added a commit to dra27/ocaml that referenced this pull request Dec 10, 2016

dra27 added a commit to dra27/ocaml that referenced this pull request Dec 10, 2016

dra27 added a commit to dra27/ocaml that referenced this pull request Dec 11, 2016

dra27 added a commit to dra27/ocaml that referenced this pull request Dec 11, 2016

dra27 added a commit to dra27/ocaml that referenced this pull request Dec 12, 2016

dra27 added a commit to dra27/ocaml that referenced this pull request Dec 14, 2016

camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.