Stack overflow on pattern matching against record

[alex35mil]

Reproduction: https://github.com/alexfedoseev/bs-stack-overflow
Problematic match: https://github.com/alexfedoseev/bs-stack-overflow/blob/master/src/X.re#L42-L85

$ bsb -clean-world -make-world
Cleaning... 2 files.
[3/3] Building src/X.cmj
FAILED: src/X.cmj src/X.cmi /Users/Alex/Dev/Sandboxes/bs-stack-overflow/src/X.bs.js
/Users/Alex/Dev/Sandboxes/bs-stack-overflow/node_modules/bs-platform/darwin/bsc.exe -bs-package-name bs-stack-overflow  -bs-package-output es6:src -color always -bs-suffix -I src -w +A  -o src/X.cmj src/X.reast
Fatal error: exception Stack overflow
FAILED: subcommand failed.
error Command failed with exit code 2.

It compiles with a lesser number of fields (try remove 1). But perf is not great though.

[yawaramin]

Would be good to see how the upstream compiler does with this pattern match.

[cristianoc]

I don't know what this config option is, but that's what triggers the stack overflow:

  "warnings": {
    "number": "+A"
  }

[alex35mil]

I’m afk but it most likely +9 which triggers it. The thing is that PM is written this way to satisfy this warning.

[yawaramin]

You can satisfy the warning also by explicitly ignoring the rest of the fields:

| {
    additionalRequirements: Pristine | Dirty(_, _),
    _, // this explicitly ignores all other fields
  } => true

[alex35mil]

Yep, but it invalidates the point of this warning, doesn't it?

[yawaramin]

In some cases, potentially yes. However in this specific code, no; because you have already specified all record fields in the first switch branch. If you add or remove any fields from the record type, that pattern will catch the change. The second pattern doesn't need to worry about it.

[bobzhang]

is this type checking failure or code generation failure?

[alex35mil]

Type checking. In the repro case there is no code generation. Though I bumped into it through generated code.

[bobzhang]

I can reproduce it and it seems to be a bug in type checking, can you narrow down it a little bit and file it upstream

[alex35mil]

You mean it’s OCaml bug and to file an issue at https://github.com/ocaml/ocaml?

[bobzhang]

@alexfedoseev yes, it is most likely an ocaml bug, but we need some way to help them reproduce it

[alex35mil]

Gotcha, yeah, will do today.

[yawaramin]

Btw, I could not reproduce this locally on OCaml native compiler ocamlopt version 4.08. Here is the code:

type visibility = Shown | Hidden

type ('outputValue, 'message) fieldStatus =
| Pristine
| Dirty of ('outputValue, 'message) result * visibility

include struct
  type message = string

  type fieldsStatuses = {
    iaasStorageConfigurations :
      iaasStorageConfigurationFieldsStatuses array;
  }

  and iaasStorageConfigurationFieldsStatuses = {
    startDate : (int, message) fieldStatus;
    term : (int, message) fieldStatus;
    rawStorageCapacity : (int, message) fieldStatus;
    diskType : (string option, message) fieldStatus;
    connectivityMethod : (string option, message) fieldStatus;
    getRequest : (int option, message) fieldStatus;
    getRequestUnit : (string option, message) fieldStatus;
    putRequest : (int option, message) fieldStatus;
    putRequestUnit : (string option, message) fieldStatus;
    transferOut : (int option, message) fieldStatus;
    transferOutUnit : (string option, message) fieldStatus;
    region : (string option, message) fieldStatus;
    cloudType : (string option, message) fieldStatus;
    description : (string option, message) fieldStatus;
    features : (string array, message) fieldStatus;
    accessTypes : (string array, message) fieldStatus;
    certifications : (string array, message) fieldStatus;
    additionalRequirements : (string option, message) fieldStatus;
  }

  type interface = { dirty : unit -> bool }

  let useForm () = {
    dirty = fun () ->
      Array.for_all
        (fun item ->
          match item with
          | {
              additionalRequirements = Pristine;
              certifications = Pristine;
              accessTypes = Pristine;
              features = Pristine;
              description = Pristine;
              cloudType = Pristine;
              region = Pristine;
              transferOutUnit = Pristine;
              transferOut = Pristine;
              putRequestUnit = Pristine;
              putRequest = Pristine;
              getRequestUnit = Pristine;
              getRequest = Pristine;
              connectivityMethod = Pristine;
              diskType = Pristine;
              rawStorageCapacity = Pristine;
              term = Pristine;
              startDate = Pristine;
            } ->
            false
          | {
              additionalRequirements = Pristine | Dirty (_, _);
              certifications = Pristine | Dirty (_, _);
              accessTypes = Pristine | Dirty (_, _);
              features = Pristine | Dirty (_, _);
              description = Pristine | Dirty (_, _);
              cloudType = Pristine | Dirty (_, _);
              region = Pristine | Dirty (_, _);
              transferOutUnit = Pristine | Dirty (_, _);
              transferOut = Pristine | Dirty (_, _);
              putRequestUnit = Pristine | Dirty (_, _);
              putRequest = Pristine | Dirty (_, _);
              getRequestUnit = Pristine | Dirty (_, _);
              getRequest = Pristine | Dirty (_, _);
              connectivityMethod = Pristine | Dirty (_, _);
              diskType = Pristine | Dirty (_, _);
              rawStorageCapacity = Pristine | Dirty (_, _);
              term = Pristine | Dirty (_, _);
              startDate = Pristine | Dirty (_, _);
            } ->
            true)
        [||]
  }
end

[cristianoc]

Does it repro on 4.06.1 ?

[yawaramin]

Cannot repro on 4.06.1:

$ ocamlopt -version
4.06.1
$ time ocamlopt pattern.ml

real	0m0.280s
user	0m0.197s
sys	0m0.065s

[alex35mil]

I didn't manage to get to my laptop today. @yawaramin Just to confirm, this is with +A enabled, right?

[yawaramin]

Agh, sorry, you're right, my mistake:

$ time ocamlopt -w +A pattern.ml 
Segmentation fault: 11

real	0m4.868s
user	0m4.253s
sys	0m0.290s

Edit: can also repro on locally installed switch 4.09.0:

$ opam switch 4.09.0
$ time ocamlopt -w +A pattern.ml
Fatal error: exception Stack overflow

real	0m5.121s
user	0m4.464s
sys	0m0.379s

[yawaramin]

There is a large backtrace from the stack overflow exception as well. the first few lines are:

Fatal error: exception Stack overflow
Raised at file "parsing/builtin_attributes.ml", line 241, characters 4-13
Called from file "typing/typecore.ml", line 2290, characters 4-156
Called from file "typing/typecore.ml", line 4411, characters 10-69
Called from file "list.ml", line 92, characters 20-23

Edit: L92 is the recursive case of List.map

[bobzhang]

moved to ocaml/ocaml#9498

[bobzhang]

will cherry pick if the fix is not too invasive