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

Assertion failed on mutual recursion with optional arguments #1001

Closed
Bastacyclop opened this issue Sep 19, 2023 · 6 comments
Closed

Assertion failed on mutual recursion with optional arguments #1001

Bastacyclop opened this issue Sep 19, 2023 · 6 comments

Comments

@Bastacyclop
Copy link

Minimal example and triggered assertion:

type t = int option

let rec f ?(optional : t) () = f ?optional ()
and g () = f ?optional:(Some 1) ()
> dune build @doc
odoc: internal error, uncaught exception:
      File "src/loader/cmi.ml", line 420, characters 21-27: Assertion failed
      Raised at Odoc_loader__Cmi.read_type_expr in file "src/loader/cmi.ml", line 420, characters 21-33
      Called from Odoc_loader__Cmt.read_pattern in file "src/loader/cmt.ml", line 40, characters 22-57
      Called from Odoc_loader__Cmt.read_value_bindings.(fun) in file "src/loader/cmt.ml", line 92, characters 18-50
      Called from Stdlib__List.fold_left in file "list.ml", line 121, characters 24-34
      Called from Odoc_loader__Cmt.read_value_bindings in file "src/loader/cmt.ml", line 86, characters 4-360
      Called from Odoc_loader__Cmt.read_structure.(fun) in file "src/loader/cmt.ml", line 583, characters 24-61
      Called from Stdlib__List.fold_left in file "list.ml", line 121, characters 24-34
      Called from Odoc_loader__Cmt.read_structure in file "src/loader/cmt.ml", line 581, characters 4-127
      Called from Odoc_loader__Cmt.read_implementation in file "src/loader/cmt.ml", line 596, characters 4-74
      Called from Odoc_loader.read_cmt in file "src/loader/odoc_loader.ml", line 145, characters 34-74
      Called from Odoc_loader.wrap_errors.(fun) in file "src/loader/odoc_loader.ml", line 164, characters 10-14
      Called from Odoc_model__Error.catch in file "src/model/error.ml", line 54, characters 21-27
      Called from Odoc_model__Error.catch_warnings.(fun) in file "src/model/error.ml", line 89, characters 18-22
      Called from Odoc_model__Error.with_ref in file "src/model/error.ml", line 67, characters 12-16
      Re-raised at Odoc_model__Error.with_ref in file "src/model/error.ml", line 72, characters 4-11
      Called from Odoc_odoc__Compile.resolve_and_substitute in file "src/odoc/compile.ml", line 87, characters 4-31
      Called from Odoc_model__Error.catch in file "src/model/error.ml", line 54, characters 21-27
      Called from Odoc_model__Error.catch_warnings.(fun) in file "src/model/error.ml", line 89, characters 18-22
      Called from Odoc_model__Error.with_ref in file "src/model/error.ml", line 67, characters 12-16
      Re-raised at Odoc_model__Error.with_ref in file "src/model/error.ml", line 72, characters 4-11
      Called from Odoc_odoc__Compile.compile.(fun) in file "src/odoc/compile.ml", line 239, characters 6-136
      Called from Cmdliner_term.app.(fun) in file "cmdliner_term.ml", line 24, characters 19-24
      Called from Cmdliner_term.app.(fun) in file "cmdliner_term.ml", line 22, characters 12-19
      Called from Cmdliner_eval.run_parser in file "cmdliner_eval.ml", line 34, characters 37-44
> dune --version
3.10.0
> odoc --version
2.2.1

Type annotations avoid hitting the assertion:

type t = int option

let rec f ?(optional : t) () = (f : ?optional:int -> unit -> unit) ?optional ()
and g () = (f : ?optional:int -> unit -> unit) ?optional:(Some 1) ()
@gpetiot
Copy link
Collaborator

gpetiot commented Oct 16, 2023

I cannot reproduce this bug with the master (2.3.0+) version of odoc (same dune version), do you still observe it after upgrading odoc?

@jonludlam
Copy link
Member

Hmm, this is still failing on 2.4.1 for me.

@jonludlam
Copy link
Member

Slightly smaller test case:

type t = int option
let rec f ?(optional : t) () = f ?optional ()

So, this turns out to be quite interesting. The problem is that we're reading the type of the function, and have found an optional argument. We then ask for the representation of the type and expect to find a arg option so we can label the argument as optional with type arg:

let arg =
            if Btype.is_optional lbl then
              match Compat.get_desc (Compat.repr arg) with
              | Tconstr(_option, [arg], _) -> read_type_expr env arg
              | _ -> assert false
            else read_type_expr env arg

In this case, we're finding the constructor t which doesn't have any arguments, so the match fails and we hit the assert. I'm not quite sure what calling repr is supposed to do - it's a no-op from OCaml 4.14 onwards, but I tried the test case on 4.12 and it failed there too - also, since I happened to have a 4.02 switch around, I tried it on that and it failed there too.

In a further test, the following did not fail:

type t = int option

let rec f ?(optional : t) () = ()

@jonludlam
Copy link
Member

It'd be handy to have someone who knows how the compiler handles this for an opinion - @Octachron, would you be able to take a look and suggest how to handle this?

@Octachron
Copy link
Member

Octachron commented Feb 16, 2024

At first glance, the current code is too syntactic. The compiler version (which may also fail in corner cases(?)) looks like this:

let extract_option_type env ty =
  match get_desc (expand_head env ty) with
    Tconstr(path, [ty], _) when Path.same path Predef.path_option -> ty
  | _ -> assert false

The important difference is the expand_head env ty part before matching the type expression description which will expand type abbreviations before trying to compare the result to the built-in option type path.

@jonludlam
Copy link
Member

Hum, I don't have an Env.t at this point. This'll require something a bit different I think.

jonludlam added a commit to jonludlam/odoc that referenced this issue Jan 9, 2025
jonludlam added a commit to jonludlam/odoc that referenced this issue Jan 13, 2025
jonludlam added a commit to jonludlam/odoc that referenced this issue Jan 13, 2025
jonludlam added a commit to jonludlam/odoc that referenced this issue Jan 13, 2025
jonludlam added a commit that referenced this issue Jan 13, 2025
jonludlam added a commit to jonludlam/opam-repository that referenced this issue Jan 23, 2025
CHANGES:

### Highlight

- Hierarchical documentation (@jonludlam, @panglesd, @Julow)
  Pages can now be organized in a directory tree structure.
  Relative and absolute references are added:
  `{!./other_page.label}`, `{!//other_page}`.

- Improved sidebar and breadcrumbs navigation (@panglesd, @gpetiot)
  The documentation pages and the libraries of the entire package are shown on
  the left sidebar.

- Added support for images, videos, audio and other assets
  The syntax is `{image!/reference/to/asset}` or `{image:URL}` for images.
  The syntax for `{video...}` and `{audio...}` is the same.
  (@panglesd, @EmileTrotignon, ocaml/odoc#1170, ocaml/odoc#1171, ocaml/odoc#1184, ocaml/odoc#1185)

- Search using Sherlodoc (@panglesd, @EmileTrotignon, @Julow)
  A new search bar that supports full-text and type-based search.

### Added

- Experimental driver (@jonludlam, @panglesd)
  The driver builds the documentation for a collection of Opam packages using
  the newer Odoc features. It supports linking external packages to ocaml.org
  and markdown files.
  This is experimental and will break in the future.

- Cross-package references (@panglesd, @Julow)
  Pages and modules from other packages can be referenced:
  `{!/otherpackage/page}`, `{!/otherpackage/Module.t}`.

- Option to remap links to other packages to ocaml.org or other site.
  See the `--remap` option of the driver or the `--remap-file` option of `odoc html-generate`.
  (@jonludlam, ocaml/odoc#1189, ocaml/odoc#1248)

- Option to compute occurrences of use of each identifiers
  The commands `aggregate-occurrences` and `count-occurrences` are added.
  (@panglesd, ocaml/odoc#976, ocaml/odoc#1076, ocaml/odoc#1206)

- Added the `odoc classify` command (@jonludlam, ocaml/odoc#1121)
  Helps driver detecting which modules belong to which libraries.
- Added `--suppress-warnings` to the CLI to remove warnings from a unit, even
  if they end up being raised in another unit through expansion
  (@jonludlam, ocaml/odoc#1260)
- Add clock emoji before `@since` tag (@yawaramin, ocaml/odoc#1089)
- Navigation for the search bar : use '/' to enter search, up and down arrows to
  select a result, and enter to follow the selected link. (@EmileTrotignon, ocaml/odoc#1088)
- Fix a big gap between the preamble and the content of a page (@EmileTrotignon, ocaml/odoc#1147)
- Add a marshalled search index consumable by sherlodoc (@EmileTrotignon, @panglesd, ocaml/odoc#1084)
- Allow referencing of polymorphic constructors in polymorphic variant type
  aliases (@panglesd, ocaml/odoc#1115)
- Added a home icon in the breacrumbs (@panglesd, ocaml/odoc#1251)
  It can be disabled with a CLI option.
- Add a frontmatter syntax for mld pages (@panglesd, ocaml/odoc#1187, ocaml/odoc#1193, ocaml/odoc#1243, ocaml/odoc#1246, ocaml/odoc#1251)
  Allows to specify the title of a page, the order of sub-pages and other
  behaviors in the sidebar.
- Added `odoc-md` to process standalone Markdown pages (@jonludlam, ocaml/odoc#1234)

### Changed

- The command line interface changed to support the new features.
  + Packages and libraries: `odoc link` must now be aware of packages and
    libraries with the `-L libname:path` and `-P pkgname:path` options. The
    module search path should still be passed with the `-I` option.
    The current package should be specified with `--current-package=pkgname`.
  + Hierarchy: `odoc compile` now outputs `.odoc` in the directory tree
    specified with `--output-dir=DIR` and the parent identifier must be
    specified with `--parent-id=PARENT`.
    The option `--source-parent-file` is removed.
  + Source code: Implementations are compiled with `compile-impl` instead of
    with `compile`. The options `--cmt=..` and `--source-name=..` are removed.
    Source code pages are generated with `html-generate-source`.
  + Assets: The commands `compile-asset`, `html-generate-asset` are added.
    The option `html-generate --asset` is removed.
  + Sidebar: The index is built using `compile-index`. The sidebar data is
    extracted from the index with `sidebar-generate` and passed to
    `html-generate --sidebar=..`.

- The syntax for `@tag` is now delimited (@panglesd, ocaml/odoc#1239)
  A `@tag` can now be followed by a paragraph or other elements.

- Updated colors for code fragments (@EmileTrotignon, ocaml/odoc#1023)
- Fixed complexity of looking up `.odoc` files (@panglesd, ocaml/odoc#1075)
- Normalize whitespaces in codespans (@gpetiot, ocaml/odoc#1085)
  A newline followed by any whitespaces is normalized as one space character.
- Reduce size of `Odoc_html_frontend` when compiled to javascript
  (@EmileTrotignon, ocaml/odoc#1072)
- Overhaul of module-type-of expansions and shadowing code (@jonludlam, ocaml/odoc#1081)
- Output file paths and labels in the man and latex backends changed to avoid
  name clashes (@Julow, ocaml/odoc#1191)

### Fixed

- Fix variant constructors being hidden if they contain hidden types
  (@jonludlam, ocaml/odoc#1105)
- Fix rare assertion failure due to optional parameters
  (@jonludlam, ocaml/odoc#1272, issue ocaml/odoc#1001)
- Fix resolution of module synopses in {!modules} lists that require --open
  (@jonludlam, ocaml/odoc#1104}
- Fix top comment not being taken from includes often enough (@panglesd, ocaml/odoc#1117)
- Fixed 404 links from search results (@panglesd, ocaml/odoc#1108)
- Fixed title content not being picked up across pages when rendering references
  (ocaml/odoc#1116, @panglesd)
- Fix wrong links to standalone comments in search results (ocaml/odoc#1118, @panglesd)
- Remove duplicated or unwanted comments with inline includes (@Julow, ocaml/odoc#1133)
- Fix bug where source rendering would cause odoc to fail completely if it
  encounters invalid syntax (@jonludlam ocaml/odoc#1208)
- Add missing parentheses in 'val (let*) : ...' (@Julow, ocaml/odoc#1268)
- Fix syntax highlighting not working for very large files
  (@jonludlam, @Julow, ocaml/odoc#1277)
jonludlam added a commit to jonludlam/opam-repository that referenced this issue Jan 23, 2025
CHANGES:

- Hierarchical documentation (@jonludlam, @panglesd, @Julow)
  Pages can now be organized in a directory tree structure.
  Relative and absolute references are added:
  `{!./other_page.label}`, `{!//other_page}`.

- Improved sidebar and breadcrumbs navigation (@panglesd, @gpetiot)
  The documentation pages and the libraries of the entire package are shown on
  the left sidebar.

- Added support for images, videos, audio and other assets
  The syntax is `{image!/reference/to/asset}` or `{image:URL}` for images.
  The syntax for `{video...}` and `{audio...}` is the same.
  (@panglesd, @EmileTrotignon, ocaml/odoc#1170, ocaml/odoc#1171, ocaml/odoc#1184, ocaml/odoc#1185)

- Search using Sherlodoc (@panglesd, @EmileTrotignon, @Julow)
  A new search bar that supports full-text and type-based search.

- Experimental driver (@jonludlam, @panglesd)
  The driver builds the documentation for a collection of Opam packages using
  the newer Odoc features. It supports linking external packages to ocaml.org
  and markdown files.
  This is experimental and will break in the future.

- Cross-package references (@panglesd, @Julow)
  Pages and modules from other packages can be referenced:
  `{!/otherpackage/page}`, `{!/otherpackage/Module.t}`.

- Option to remap links to other packages to ocaml.org or other site.
  See the `--remap` option of the driver or the `--remap-file` option of `odoc html-generate`.
  (@jonludlam, ocaml/odoc#1189, ocaml/odoc#1248)

- Option to compute occurrences of use of each identifiers
  The commands `aggregate-occurrences` and `count-occurrences` are added.
  (@panglesd, ocaml/odoc#976, ocaml/odoc#1076, ocaml/odoc#1206)

- Added the `odoc classify` command (@jonludlam, ocaml/odoc#1121)
  Helps driver detecting which modules belong to which libraries.
- Added `--suppress-warnings` to the CLI to remove warnings from a unit, even
  if they end up being raised in another unit through expansion
  (@jonludlam, ocaml/odoc#1260)
- Add clock emoji before `@since` tag (@yawaramin, ocaml/odoc#1089)
- Navigation for the search bar : use '/' to enter search, up and down arrows to
  select a result, and enter to follow the selected link. (@EmileTrotignon, ocaml/odoc#1088)
- Fix a big gap between the preamble and the content of a page (@EmileTrotignon, ocaml/odoc#1147)
- Add a marshalled search index consumable by sherlodoc (@EmileTrotignon, @panglesd, ocaml/odoc#1084)
- Allow referencing of polymorphic constructors in polymorphic variant type
  aliases (@panglesd, ocaml/odoc#1115)
- Added a home icon in the breacrumbs (@panglesd, ocaml/odoc#1251)
  It can be disabled with a CLI option.
- Add a frontmatter syntax for mld pages (@panglesd, ocaml/odoc#1187, ocaml/odoc#1193, ocaml/odoc#1243, ocaml/odoc#1246, ocaml/odoc#1251)
  Allows to specify the title of a page, the order of sub-pages and other
  behaviors in the sidebar.
- Added `odoc-md` to process standalone Markdown pages (@jonludlam, ocaml/odoc#1234)

- The command line interface changed to support the new features.
  + Packages and libraries: `odoc link` must now be aware of packages and
    libraries with the `-L libname:path` and `-P pkgname:path` options. The
    module search path should still be passed with the `-I` option.
    The current package should be specified with `--current-package=pkgname`.
  + Hierarchy: `odoc compile` now outputs `.odoc` in the directory tree
    specified with `--output-dir=DIR` and the parent identifier must be
    specified with `--parent-id=PARENT`.
    The option `--source-parent-file` is removed.
  + Source code: Implementations are compiled with `compile-impl` instead of
    with `compile`. The options `--cmt=..` and `--source-name=..` are removed.
    Source code pages are generated with `html-generate-source`.
  + Assets: The commands `compile-asset`, `html-generate-asset` are added.
    The option `html-generate --asset` is removed.
  + Sidebar: The index is built using `compile-index`. The sidebar data is
    extracted from the index with `sidebar-generate` and passed to
    `html-generate --sidebar=..`.

- The syntax for `@tag` is now delimited (@panglesd, ocaml/odoc#1239)
  A `@tag` can now be followed by a paragraph or other elements.

- Updated colors for code fragments (@EmileTrotignon, ocaml/odoc#1023)
- Fixed complexity of looking up `.odoc` files (@panglesd, ocaml/odoc#1075)
- Normalize whitespaces in codespans (@gpetiot, ocaml/odoc#1085)
  A newline followed by any whitespaces is normalized as one space character.
- Reduce size of `Odoc_html_frontend` when compiled to javascript
  (@EmileTrotignon, ocaml/odoc#1072)
- Overhaul of module-type-of expansions and shadowing code (@jonludlam, ocaml/odoc#1081)
- Output file paths and labels in the man and latex backends changed to avoid
  name clashes (@Julow, ocaml/odoc#1191)

- Fix variant constructors being hidden if they contain hidden types
  (@jonludlam, ocaml/odoc#1105)
- Fix rare assertion failure due to optional parameters
  (@jonludlam, ocaml/odoc#1272, issue ocaml/odoc#1001)
- Fix resolution of module synopses in {!modules} lists that require --open
  (@jonludlam, ocaml/odoc#1104}
- Fix top comment not being taken from includes often enough (@panglesd, ocaml/odoc#1117)
- Fixed 404 links from search results (@panglesd, ocaml/odoc#1108)
- Fixed title content not being picked up across pages when rendering references
  (ocaml/odoc#1116, @panglesd)
- Fix wrong links to standalone comments in search results (ocaml/odoc#1118, @panglesd)
- Remove duplicated or unwanted comments with inline includes (@Julow, ocaml/odoc#1133)
- Fix bug where source rendering would cause odoc to fail completely if it
  encounters invalid syntax (@jonludlam ocaml/odoc#1208)
- Add missing parentheses in 'val (let*) : ...' (@Julow, ocaml/odoc#1268)
- Fix syntax highlighting not working for very large files
  (@jonludlam, @Julow, ocaml/odoc#1277)
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

No branches or pull requests

4 participants