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.

Sign up for GitHub

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

Jump to bottom

-dparsed produces invalid BSV module function that takes another module as an argument #663

Open
RyanGlScott opened this issue Jan 14, 2024 · 2 comments · May be fixed by #666
Open

-dparsed produces invalid BSV module function that takes another module as an argument #663

RyanGlScott opened this issue Jan 14, 2024 · 2 comments · May be fixed by #666

Comments

@RyanGlScott
Copy link
Contributor

Given this BSV code:

package Foo;

module [Module] helloWorld#(Module#(Empty) m)(Empty);
  let e <- m;
endmodule

endpackage

Compiling it with bsc -dparsed=FooParsed.bsv Foo.bsv yields:

package Foo;
module helloWorld#(Module#(Empty) m)(Empty);
  let e <- m;
endmodule: helloWorld

endpackage: Foo

This is almost the same code, but there is one key difference: the module keyword lacks the [Module] part after it. This change is significant enough to cause bsc to reject it:

$ bsc FooParsed.bsv 
Warning: Unknown position: (S0001)
  File name `FooParsed.bsv' does not match package name `Foo'
Error: "FooParsed.bsv", line 2, column 8: (T0029)
  Signature mismatch (given too general):
  given:
  function a__#(Empty) f(Module#(Empty) x1)   provisos (IsModule#(a__, b__))
  deduced:
  module f#(Module#(Empty) x1)(Empty)   provisos (IsModule#(Module, Id__))

Any reason not to preserve the [Module] part?
@quark17
Copy link
Collaborator

quark17 commented Jan 17, 2024

Thanks for reporting these. There's been no effort to make sure that the pretty-printer for an entire BSV file will read back in correctly and identically, so I am not surprised if you find issues like this. The only concern was the printing of any snippets that appear in error messages to the user, which are usually just types, or IDs, or individual method calls. The -d dump flags were considered hidden features for developers, and they didn't need to be perfect (and could change or disappear at any time). But it's a good aim to have the pretty-printer work for full programs, so thank you for reporting this.

The module type should be preserved. I assume that this is just a bug in CVPrint.hs, probably around line 165, in the function p2defs:

line1 = t"module" <+> pvpId d i <> f ys <> t"(" <> pvPrint d 0 ity <> t")"

@RyanGlScott
Copy link
Contributor Author

Thanks! And I certainly don't expect the pretty-printer to be 100% accurate in all cases. My use case is that I am generating very simple BSV code programmatically using the bsc source code, and I ran into some situations where even very simple code fails to parse when fed back into bsc. My goal is to fix only those issues that I encounter when pretty-printing the very limited subset of BSV that I'm generating.

I'll give CVPrint a closer look. For posterity's sake, here is a permalink to the code mentioned in #663 (comment):

bsc/src/comp/CVPrint.hs

Line 165 in e4361d9

line1 = t"module" <+> pvpId d i <> f ys <> t"(" <> pvPrint d 0 ity <> t")"

RyanGlScott added a commit to RyanGlScott/bsc that referenced this issue Jan 18, 2024
@RyanGlScott
BSV: Pretty-print module return types when possible
54028fa 
The BSV pretty-printer would print a function like this:

```
module [Module] helloWorld#(Module#(Empty) m)(Empty);
  let e <- m;
endmodule
```

Without the `[Module]` bit, resulting in a function declaration that wouldn't
typecheck. This patch makes a best-effort attempt to pretty-print this bit
whenever possible. More specifically:

* We check if the return type of a function is equal to `M ty`, where `M` is a
  type constructor like `Module`. If so, pretty-print `[M]`.
* Otherwise, check if the return type is equal to `m ty`, where `m` is a type
  variable with a corresponding `IsModule#(m, c)` constraint. If so,
  pretty-print `[m]`.

  The `findModId` function is responsible for finding type variables like `m`.
  While investigating this issue, I noticed a bug in which `findModId` would
  drop the `IsModule#(m, c)` constraint in which `m` appears, which would cause
  the constraint not to be pretty-printed. I've fixed this bug as part of this
  patch.

Fixes B-Lang-org#663.
@RyanGlScott RyanGlScott linked a pull request Jan 18, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

BSV: Pretty-print module return types when possible RyanGlScott/bsc
2 participants
@RyanGlScott @quark17