Skip to content

Commit

Permalink
Add prefix "page-" to target name when adding page content to cache
Browse files Browse the repository at this point in the history
This PR adds prefix "page-" to the target name when adding a page content to the cache.

* In `lookup_page` function, it adds prefix "page-" to the target name when lookup.

* However, when adding a page content to the cache (i.e. `build_env_for_page` -> `build` -> `add_unit_to_cache`), it did not add the prefix.

This discrepancy introduced an incorrect reference resolution when there is the same name of page and module. For example, suppose we have a module `Foo` and a page `Foo.mld` and `Foo.mld` includes a reference to `Foo`, i.e. `{!module-Foo}`.

1. It invokes `build_env_for_page` for the page `Foo.mld`. Now, the cache has a mapping for `Foo`, but its value includes the page content only.

2. When resolving `{!module-Foo}`, it tries to find it from the cache. It found, but the result is not for the unit contents.

3. It fails to make a correct link in `Foo.mld`.
  • Loading branch information
skcho committed Dec 6, 2022
1 parent 625423a commit 8a9cfef
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
7 changes: 6 additions & 1 deletion src/odoc/resolver.ml
Expand Up @@ -190,7 +190,12 @@ let lookup_page ap target_name =

(** Add the current unit to the cache. No need to load other units with the same
name. *)
let add_unit_to_cache u = Hashtbl.add unit_cache (unit_name u) [ u ]
let add_unit_to_cache u =
let target_name =
(match u with Odoc_file.Page_content _ -> "page-" | Unit_content _ -> "")
^ unit_name u
in
Hashtbl.add unit_cache target_name [ u ]

type t = {
important_digests : bool;
Expand Down
6 changes: 2 additions & 4 deletions test/xref2/github_issue_917.t/run.t
@@ -1,16 +1,14 @@
# References to pages and items when page and module names are the same.

$ compile foo.mld foo.mli
File "foo.mld", line 5, characters 7-20:
Warning: Failed to resolve reference unresolvedroot(foo) Parent_module: Lookup failure (root module): foo

$ jq_scan_references() { jq -c '.. | .["`Reference"]? | select(.) | .[0]'; }

Every references in `page-foo.odocl` should resolve:

$ odoc_print page-foo.odocl | jq_scan_references
{"`Resolved":{"`Identifier":{"`LeafPage":[{"Some":{"`Page":["None","test"]}},"foo"]}}}
{"`Root":["foo","`TModule"]}
{"`Resolved":{"`Identifier":{"`Root":[{"Some":{"`Page":["None","test"]}},"Foo"]}}}
{"`Resolved":{"`Identifier":{"`Root":[{"Some":{"`Page":["None","test"]}},"Foo"]}}}
{"`Resolved":{"`Identifier":{"`LeafPage":[{"Some":{"`Page":["None","test"]}},"foo"]}}}

Every references in `foo.odocl` should resolve:
Expand Down

0 comments on commit 8a9cfef

Please sign in to comment.