ocaml icon indicating copy to clipboard operation
ocaml copied to clipboard

Option to disable -principal mode

Open vicuna opened this issue 13 years ago • 17 comments

Original bug ID: 5561 Reporter: @alainfrisch Status: confirmed (set by @damiendoligez on 2013-12-05T14:39:06Z) Resolution: open Priority: low Severity: feature Category: typing

Bug description

It would be convenient to add a -noprincipal flag to the compiler. With OMake for instance, it would make it easy to add -principal to OCAMLFLAGS globally for the project, and then append -noprincipal to OCAMLFLAGS for some directories.

vicuna avatar Mar 27 '12 13:03 vicuna

Comment author: @gasche

So you would call ocamlc -principal -noprincipal or ocamlc -noprincipal -principal, and expect it to disable principality checks? That looks a little weird to me (I like to think that command-line arguments are declarative rather than imperative).

vicuna avatar Mar 27 '12 14:03 vicuna

Comment author: @alainfrisch

The last option would take precedence, as for warnings. I find it quite convenient to be able to define a "OCAMLFLAGS" globally, and adapt it locally for specific files or directories, by appending some extra options.

vicuna avatar Mar 27 '12 15:03 vicuna

Comment author: @protz

What about an optional parameter after -principal?

  • -principal ? current behavior
  • -principal yes ? current behavior
  • -principal no ? overrides any previous values for the argument.

That sounds like a valid use-case for a big codebase.

vicuna avatar Mar 29 '12 08:03 vicuna

Comment author: @gasche

Regarding the interface, I rather prefer Alain's proposal.

The change would however raise the question of doing this for other options: -norectypes, -nostrict-sequence, -stdlib, -assert, -nounsafe... (But we already have -nolabels and -labels that (if I understand correctly) do are not inverse one of another.)

I'm not sure the case for warning makes for a good precedent (no pun intended), but if it's decided to add -noprincipal I really don't see why we shouldn't have -norectypes as well.

vicuna avatar Mar 29 '12 09:03 vicuna

Comment author: @alainfrisch

-rectypes is quite viral: if you compile a module with it, you need to compile all other modules that depend on it with -rectypes as well. So it's rather unlikely that -rectypes would be the default for a big project, with some "-norectypes" exceptions.

I also don't expect -unsafe to be the default for a big code base. It could be turned on for performance-critical and carefully verified code (but in my experience, it has a rather low impact, so I'm not even sure it is used at all).

A -nostrict-sequence would probably make sense.

vicuna avatar Mar 29 '12 09:03 vicuna

@alainfrisch Given there has been no progress on this issue for 8 years, would you like to perhaps implement the flag (I agree that positional precedence seems fine, but watch out for OCAMLPARAM) or close this issue?

mshinwell avatar Apr 20 '20 16:04 mshinwell

The original feature request sounds like a good newcomer job, marking it as such.

gasche avatar Apr 20 '20 16:04 gasche

There's no point in spending newcomer time on features people don't actually want, though. (I'm not certain that is the case for this one, but it seems likely...)

mshinwell avatar Apr 21 '20 08:04 mshinwell

There is a tension here: things that are very important to a core developer are not useful "newcomer jobs" either because they get done quickly. It is actually useful for onboarding new people to have issues or feature requests that are not urgently important, and also not too difficult or controversial.

gasche avatar Apr 21 '20 09:04 gasche

FWIW, I don't really care about the feature any more, and I'm certainly not against closing it. Re-assigning to @gasche to let you decide whether it's good to keep this open as a "newcomer job".

alainfrisch avatar Jun 03 '20 19:06 alainfrisch

In general, it would be good to have every flag come in pairs to disable/enable. That makes it easier to do all kinds of build-system hacks, and it also makes it easier to change the default later.

The newcomer job could be to identify the flags that don't have their negative, and to add all the missing ones. It's not so trivial because of the OCAMLPARAM business.

damiendoligez avatar Nov 02 '20 14:11 damiendoligez

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

github-actions[bot] avatar Aug 03 '22 04:08 github-actions[bot]

Still a good "first issue".

damiendoligez avatar Sep 07 '22 15:09 damiendoligez

Damien Doligez (2022/09/07 08:10 -0700):

Still a good "first issue".

In a similar way, I would like to have a -no-absname option and, more generally, I think it would be both helpful and principled to have a way to define boolean arguemnt in a way that ensures that for every option -foo we do have a -no-foo counterpart.

Of course, one possibility would be to introduce functions to add the two options to the specification list, but that would loose the information that the two options are related, whereas it's an information one may want ot have when displaying the help text.

One other thing I'd see as desirable is the ability to isolate the deprecated options and to associate to each option the version from which the option has started to be deprecated.

All this suggests that it would be nice to have a higher level, more abstract way of describing command-line arguments.

shindere avatar Sep 13 '22 13:09 shindere

I think a function returning the two options in a list would be a good first step.

gasche avatar Sep 13 '22 15:09 gasche

Gabriel Scherer (2022/09/13 08:04 -0700):

I think a function returning the two options in a list would be a good first step.

Yes, probably. It has the merit of not touching the representation.

Still, doing that means it will become easier to add boolean options, without making the help text look nicer, which is a sad thing.

shindere avatar Sep 13 '22 15:09 shindere

I don't understand your remark about the help text. We generate the options, so we could choose for example to always set the help text of the -no-... option to "(negation of the option ...)" or something.

Currently we show for example:

  -principal  Check principality of type inference
  -no-principal  Do not check principality of type inference (default)

I think we could have a function generating exactly that, possibly by taking the two help descriptions in arguments (... but (default) could be added programmatically). If we decide to use a different convention to advertise these options, maybe we can still implementing it as a functional helper. I would first try to decide the output we want, and only then think about the implmentation.

gasche avatar Sep 13 '22 15:09 gasche

Gabriel Scherer (2022/09/13 08:54 -0700):

I don't understand your remark about the help text. We generate the options, so we could choose for example to always set the help text of the -no-... option to "(negation of the option ...)" or something.

Sure. It's just that I had the thought that two boolean options could be displayed together in the help text, rather than as two separate options. It'd feel more readable to me that way and with the explanation of which one is the default.

Currently we show for example:

  -principal  Check principality of type inference
  -no-principal  Do not check principality of type inference (default)

Yeah and I imagined these two could be merged in only one line...

I think we could have a function generating exactly that, possibly by taking the two help descriptions in arguments (... but (default) could be added programmatically). If we decide to use a different convention to advertise these options, maybe we can still implementing it as a functional helper. I would first try to decide the output we want, and only then think about the implmentation.

Yes let's agree on the output, you are right!

shindere avatar Oct 11 '22 09:10 shindere

Just to link it in, I think the original issue was addressed in 2016 in #514

dra27 avatar Oct 21 '22 11:10 dra27