ocaml · brendanzab · Nov 13, 2024 · Leonidas-from-XIV · Nov 18, 2024 · brendanzab
diff --git a/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/dune b/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/dune
@@ -0,0 +1,4 @@
+(include_subdirs qualified)
+
+(executable
+ (name foo))
diff --git a/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/dune-project b/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/dune-project
@@ -0,0 +1,2 @@
+(lang dune 3.14)
+(using menhir 2.1)
diff --git a/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/foo.ml b/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/foo.ml
@@ -0,0 +1,2 @@
+let () =
+  assert (Lang.Parser.expr (fun _ -> Lang.Parser.EOF) (Lexing.from_string "") = Lang.Ast.Unit)
diff --git a/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/lang/ast.ml b/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/lang/ast.ml
@@ -0,0 +1,2 @@
+type expr =
+  | Unit
diff --git a/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/lang/dune b/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/lang/dune
@@ -0,0 +1,2 @@
+(menhir
+ (modules parser))
diff --git a/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/lang/parser.mly b/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/lang/parser.mly
@@ -0,0 +1,8 @@
+%token EOF
+
+%start <Ast.expr> expr
+
+%%
+
+expr:
+| EOF { Ast.Unit }
diff --git a/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/run.t b/test/blackbox-tests/test-cases/menhir/include-subdirs-siblings.t/run.t
@@ -0,0 +1,6 @@
+Menhir parsers in qualified subdirectories should be able to refer to sibling modules:
+
+  $ dune build
+  File "lang/parser.mly", line 3, characters 8-16:
+  Error: Unbound module Ast
+  [1]
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		let () =
		assert (Lang.Parser.expr (fun _ -> Lang.Parser.EOF) (Lexing.from_string "") = Lang.Ast.Unit)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		(menhir
		(modules parser))
Copy link Collaborator Leonidas-from-XIV Nov 18, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. From what I see the error message is correct, as this makes the `Ast` module invisible to Menhir. Copy link Author brendanzab Nov 18, 2024 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Oh, why is that? I had assumed it would work similarly to other qualified modules - where you could mention sibling modules from within a subdirectory? See the directory tree from #11119: `├── dune ├── dune-project ├── foo.ml ├── lang │ ├── ast.ml │ ├── dune │ └── parser.mly └── run.t` If this is intentional is there a workaround for this, or is this pattern supposed to be impossible with Menhir? Copy link Author brendanzab Nov 18, 2024 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I’ve also tried the following from the root `dune` file: `(subdir lang (menhir (modules parser)))` Which resulted in the same error. I also tried: `(menhir (modules lang/parser))` Which resulted in: `Error: dune__exe__Lang/parser corresponds to an invalid module name -> required by _build/default/.foo.eobjs/dune__exe.ml-gen -> required by _build/default/.foo.eobjs/byte/dune__exe.cmi -> required by _build/default/lang/parser__mock.mli.inferred -> required by _build/default/lang/parser.ml -> required by alias lang/all -> required by alias default` Copy link Author brendanzab Nov 20, 2024 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Would you prefer it if I used an example that used one of the approaches above, or are those also intended behavior? I think the main reason I used a `dune` file in the subdirectory was because I had no other way of referencing the `.mly` from the root directory via the `menhir` stanza. And I assume using the `subdir` stanza is isomorphic to the approach I am currently using (but I could be mistaken on that). Copy link Author brendanzab Nov 20, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I’ve also tried variations of `(modules Lang.Parser)`, `(modules Lang__Parser)`, `(modules dune__exe__Lang__parser)` to no avail in the `menhir` stanza. Copy link Author brendanzab Dec 14, 2024 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. @Leonidas-from-XIV If it helps, it seems #8987 was merged, which seems like a case where the `.mly` is the “group interface” of the subdirectory, which references a sibling module: `├── dune ├── dune-project └── group ├── dune (menhir stanza is here) ├── group.mly (refers to M) └── m.ml` https://github.com/ocaml/dune/pull/8987/files In contrast the pattern I am after is: `├── dune ├── dune-project └── group ├── dune (menhir stanza is here) ├── m.ml └── parser.ml (refers to M)` Not sure if that test is enough to covers this use-case case as well, as in this case the `parser.mly` is not the group interface? But it’d be really cool if that issue was fixed, this pattern would be supported as well!