Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007408OCamlmiddle end (typedtree to clambda)public2016-11-11 19:012018-09-04 06:41
Assigned Tonojebar 
PlatformOSOS Version
Product Version4.03.0 
Target Version4.08.0+dev/beta1Fixed in Version 
Summary0007408: Compile-time fatal error when we write external definitions with wrong signatures
DescriptionBoth, byte- and native-code compiler. 4.03 and 4.04 are affected.
Additional Information? /tmp cat
external f: 'a -> 'b -> 'a = "%identity"

let () = print_endline @@ f "a" 1
? /tmp ocamlc
>> Fatal error: Bytegen.comp_primitive
Fatal error: exception Misc.Fatal_error
Attached Files

- Relationships
related to 0006151closed Misc.Fatal_error when using "%apply" for more arguments. 
has duplicate 0007846resolvednojebar incorrect arity on primitives causes assertion failure 

-  Notes
xleroy (administrator)
2016-11-11 19:41

At least you get a compile-time error -- even if it is not pretty. I mean, it would be much worse to silently generate wrong code.

My first reaction to this PR is "just don't do that". External declarations with "%" primitives are not documented and are normally used only in the standard library.

I move to "close/no change required" this PR.
gasche (administrator)
2016-11-12 03:32

Xavier, I agree that it's fine to have a compiler error on this case, but I also think that an uncaught exception is not a very nice way to stop the compilation. I quickly looked at the backtrace and it is not at all obvious, to me, why it is failing at this point of the compilation (it is failing during code generation, not before, in a different path in ocamlc and ocamlopt).

I don't think that anyone should prioritize spending time on this, but (1) having a nicer error message could be nice and (2) the uncaught exception may be the symptom of something fragile in the compiler that could manifest itself in worse ways in other occasions.

I would thus be tempted to keep the PR open with a "junior job" tag: I think that understanding what is happening by investigating the trace, and reporting on whether there is a satisfying change that would improve the behavior, could be a nice onboarding task for someone interested in learning more about the compiler.
xleroy (administrator)
2016-12-05 09:39

We need to grow a sense of priority. Reclassifying as "feature wish".
shinwell (developer)
2016-12-07 19:01

I've closed I think two other PRs today that were related to the calling of % primitives, saying basically what Xavier first wrote above. I don't think these failures are a sign of something more deeply wrong; it's just that the arguments aren't checked until fairly late in the compilation pipeline.

I move that we close this PR as well, together if needs be adding a new PR for adding to the stdlib bindings for any %-primitives that are particularly heavily used.
shinwell (developer)
2016-12-07 19:03

(As for getting newcomers up to speed and priorities: I can't help but think there are straightforward issues to be fixed that will provide more value for everyone than things like this.)
gasche (administrator)
2016-12-07 23:19

Well if three separate people were surprised enough by the behavior to create a PR, I think it is safe to assume that many people encounter these issues through their normal development workflow and are surprised by what is happening. I would say it confirms that this is a non-null usability issue.
lpw25 (developer)
2016-12-08 10:30

I think I agree with Gabriel that we should check the arity of primitives explicitly in translcore. It leaves me a little uneasy that the middle-ends can have primitives with the wrong number of arguments going through them (since it is not checked until cmmgen). I'm not entirely convinced that you can't get broken behaviour rather than a compile-time error.

As a side-note, does anyone know why the "%loc_*" primitives are supported at both 0 and 1 arity in `transl_primitive`? The 1 arity behaviour seems very odd and I have no idea why it is there.
gasche (administrator)
2016-12-08 19:42

The same primitive name is used for __LOC__, which returns its own location, and (__LOC_OF__ foo) which returns the location of (foo). I think this is very ugly, but I was the only one voicing that opinion when the feature was discussed (caml-devel, "add a new primitive to return location at call position", march-april-may 2014).
smuenzel-js (reporter)
2018-09-03 10:41

I agree that not enforcing the arity throughout all the phases may end up making it easier to introduce other, related bugs in the future.

What about encoding this in the type

type 'a prim =
| Prim1 of { primitive : prim1; arg : 'a }
| Prim2 of { primitive : prim2; arg0 : 'a; arg1 : 'a }
or something similar.
nojebar (developer)
2018-09-04 06:40

Arities of %-primitives are now checked following the merge of [^]

- Issue History
Date Modified Username Field Change
2016-11-11 19:01 Kakadu New Issue
2016-11-11 19:41 xleroy Note Added: 0016542
2016-11-11 20:04 xleroy Severity crash => minor
2016-11-11 20:04 xleroy Status new => acknowledged
2016-11-12 03:32 gasche Note Added: 0016558
2016-11-12 18:08 gasche Tag Attached: junior_job
2016-12-05 09:39 xleroy Note Added: 0016632
2016-12-05 09:39 xleroy Severity minor => feature
2016-12-05 09:39 xleroy Summary Crashes when we write external definitions with wrong signatures => Compile-time fatal error when we write external definitions with wrong signatures
2016-12-07 10:46 xleroy Relationship added related to 0006151
2016-12-07 19:01 shinwell Note Added: 0016802
2016-12-07 19:03 shinwell Note Added: 0016803
2016-12-07 23:19 gasche Note Added: 0016810
2016-12-08 10:30 lpw25 Note Added: 0016832
2016-12-08 19:42 gasche Note Added: 0016928
2017-02-18 09:49 xleroy Target Version => later
2017-02-23 16:36 doligez Category OCaml general => -OCaml general
2017-02-27 16:02 doligez Category -OCaml general => middle end (typedtree to clambda)
2018-09-03 09:11 yallop Relationship added has duplicate 0007846
2018-09-03 10:41 smuenzel-js Note Added: 0019333
2018-09-04 06:40 nojebar Note Added: 0019339
2018-09-04 06:41 nojebar Target Version later => 4.08.0+dev/beta1
2018-09-04 06:41 nojebar Status acknowledged => resolved
2018-09-04 06:41 nojebar Resolution open => fixed
2018-09-04 06:41 nojebar Assigned To => nojebar

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker