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

Deprecate the -thread and -vmthread flags #7687

Closed
vicuna opened this issue Dec 6, 2017 · 7 comments
Closed

Deprecate the -thread and -vmthread flags #7687

vicuna opened this issue Dec 6, 2017 · 7 comments
Assignees
Milestone

Comments

@vicuna
Copy link

vicuna commented Dec 6, 2017

Original bug ID: 7687
Reporter: @dbuenzli
Assigned to: @nojb
Status: resolved (set by @nojb on 2018-04-17T10:55:58Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.06.0
Target version: 4.07.0+dev/beta2/rc1/rc2
Category: compiler driver
Monitored by: @nojb @gasche @yallop @dbuenzli

Bug description

The manual [0] mandates these flags to be used for any compilation unit one wants to compile against the thread or vmthread libraries which entails library specific compilation logic to be implemented by build systems.

I turns out (see discussion in [1]) that these flags currently have no effect beyond altering the search path for includes and libraries which can perfectly be handled by the scheme(s) we have for compiling against a given library.

Further discussion in [1] seems to indicate there is no strong wish to change that state of affairs.

As such it would be good if

  1. These flags are marked as deprecated in the compiler driver
  2. The documentation is updated to no longer mandate these flags to be added on the cli when the threads or vmthread library are used.

[0] http://caml.inria.fr/pub/docs/manual-ocaml/libthreads.html
[1] #1504

@vicuna
Copy link
Author

vicuna commented Dec 6, 2017

Comment author: @nojb

Looked at this. The simplest thing is to make the flag -(vm)thread equivalent to -I +(vm)threads and remove Clflags.use_thread, Clflags.use_vmthread.

This introduces a tiny difference in the ordering of include directories:

  • The -(vm)thread option adds the include directory +(vm)threads first (or last, do not recall at the moment), while -I +(vm)threads will add it in the order in which it appears with respect to the other -I options.

And a (small) breaking change:

Opinions? We can of course maintain Clflags.use_thread and Clflags.use_vmthread and simply do not use them in the compiler codebase to avoid the breaking change, but personally do not think it is worth the trouble.

@vicuna
Copy link
Author

vicuna commented Dec 6, 2017

Comment author: @dbuenzli

nojebar I was actually rather thinking about all this being a purely documentation issue. Indicate that the flags are deprecated in ocamlc -help and remove the rules mentioned in the Threads chapter of the manual.

I don't think it's worth breaking all the build systems out there.

@vicuna
Copy link
Author

vicuna commented Dec 6, 2017

Comment author: @nojb

Agree. The command line options would be left as they are (suitably documented), but there is also the issue that the flags are exported via Clflags.use_vmthread and Clflags.use_thread to compiler-libs users, which is what my note was about.

In other words, there is a choice between doing: 1) "just documentation", and 2) "just documentation" + cleanup of compiler codebase (i.e. removal of Clflags.use_vmthread, Clflags.use_thread).

@vicuna
Copy link
Author

vicuna commented Dec 20, 2017

Comment author: @xavierleroy

I would make a distinction between VM threads and system threads.

VM threads come with their own implementation of parts of the standard library (incl. stdlib.cma), so it felt important to me to tell users that all files of a VM-thread project should be compiled with -vmthread, so that they all agree on the standard library used. [ This said, it's only the implementations of some stdlib modules that change, not the interfaces, so maybe it's not that bad? ] For this reason, I'd keep the -vmthread flag documented and not deprecated until we get rid of VM threads altogether.

System threads, on the other hand, don't play tricks with the stdlib, so I'm OK with documenting -thread as not required and equivalent to -I +threads.

@vicuna
Copy link
Author

vicuna commented Mar 8, 2018

Comment author: @nojb

#1653

@vicuna vicuna closed this as completed Apr 17, 2018
@vicuna
Copy link
Author

vicuna commented Apr 17, 2018

Comment author: @gasche

I hit an issue recently where a program didn't link correctly because the -thread option was not passed to ocamlfind. I think it's fine to deprecate the -thread option of the compiler, but we may live with it in the ocamlfind-world for the time being.

@vicuna
Copy link
Author

vicuna commented Sep 10, 2018

Comment author: @dbuenzli

This PR deprecates vmthread #2038

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

No branches or pull requests

2 participants