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 support for flambda's optimization options to OCamlbuild #68

Merged
merged 1 commit into from
Apr 6, 2016

Conversation

martin-neuhaeusser
Copy link
Contributor

Add support for the new optimization command line options brought by flambda.
The OCamlbuild flags are named according to the flambda options with two exceptions (to clarify the meaning of the flag):

  • -rounds <int> can be specified with the optimization_rounds(int) flag
  • -O{2,3,classic} can be specified with the optimize({2,3,classic}) flag

@gasche
Copy link
Member

gasche commented Apr 6, 2016

Does your choice of documentation parameter reflect the current default values? Will we want to keep them in sync' in the future?

@gasche
Copy link
Member

gasche commented Apr 6, 2016

The patch is fine as far as I'm concerned, I'll merge it later if there is no other comments.

Just, could you cut the lines above 80 columns? (One certainly is, I can't browse it in the github diff, I suspect some of the flags call may be as well). We should check that it is documented, but it's better to keep respecting this old rule.

@martin-neuhaeusser
Copy link
Contributor Author

Yes, the default values correspond to those documented here.
I will adjust the overlong columns in a minute (but there are plenty of other lines in the file which are longer than 80 characters - I won't touch those).

@gasche
Copy link
Member

gasche commented Apr 6, 2016

While you're at it, you should also add a Changes entry. I just pushed a new section for the upcoming release, you'll see it if you rebase on your end.

@martin-neuhaeusser
Copy link
Contributor Author

Did the changes and rebased into a single commit.

@gasche
Copy link
Member

gasche commented Apr 6, 2016

The Changes option should go in the next section. I'll cleanup myself (I've been nitpicky enough already), add an information on option naming in the Changes entry (it also serves as user documentation), and update the manual. Thanks!

@gasche gasche merged commit 8874db7 into ocaml:master Apr 6, 2016
nilsbecker added a commit to nilsbecker/manual-ocamlbuild that referenced this pull request Apr 27, 2016
Used defaults as per ocaml/ocamlbuild#68 . Please check if this makes sense, I did not test or verufy.
nilsbecker pushed a commit to nilsbecker/ocamlbuild that referenced this pull request May 6, 2016
in a little section; defaults for the values were copied from ocaml#68 .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants