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

Make compile targets a subcommand instead of a flag #2700

Merged
merged 40 commits into from
Apr 9, 2024

Conversation

janmasrovira
Copy link
Collaborator

@janmasrovira janmasrovira commented Mar 25, 2024

Changes

The main goal of this pr is to remove the --target flag for juvix compile and use subcommands instead. The targets that are relevant to normal users are found in juvix compile --help. Targets that are relevant only to developers are found in juvix dev compile --help.

Below I list some of the changes in more detail.

Compile targets for user-facing languages

  • juvix compile native
  • juvix compile wasi. I wasn't sure how to call this: wasm, wasm32-wasi, etc. In the end I thought wasi was short and accurate, but we can change it.
  • juvix compile vampir
  • juvix compile anoma
  • juvix compile cairo

New compile targets for internal languages

See juvix dev compile --help.

  1. dev compile core has the same behaviour as dev core from-concrete. The dev core from-concrete is redundant at the moment.
  2. dev compile tree compiles to Tree and prints the InfoTable to the output file wihout any additional checks.
  3. dev compile reg compiles to Reg and prints the InfoTable to the output file wihout any additional checks.
  4. dev compile asm compiles to Asm and prints the InfoTable to the output file wihout any additional checks.
    1. dev compile casm compiles to Asm and prints the Result to the output file wihout any additional checks. TODO: should the Result be printed or something else? At the moment the Result lacks a pretty instance.

Optional input file

  1. The input file for commands that expect a .juvix file as input is now optional. If the argument is ommited, he main file given in the package.yaml will be used. This applies to the following commands:
    1. juvix compile [native|wasi|geb|vampir|anoma|cairo]
    2. juvix dev compile [core|reg|tree|casm|asm]
    3. juvix html
    4. juvix markdown.
    5. juvix dev internal [typecheck|pretty].
    6. juvix dev [parse|scope]
    7. juvix compile [native|wasi|geb|vampir|anoma|cairo]
    8. note that juvix format has not changed its behaviour.

Refactor some C-like compiler flags

Both juvix compile native and juvix compile wasi support --only-c (-C), --only-preprocess (-E), --only-assemble (-S). I propose to deviate from the gcc style and instead use a flag with a single argument:

  • --cstage [source|preprocess|assembly|exec(default)]. I'm open to suggestions. For now, I've kept the legacy flags but marked them as deprecated in the help message.

Remove code duplication

I've tried to reduce code duplication. This is sometimes in tension with code readability so I've tried to find a good balance. I've tried to make it so we don't have to jump to many different files to understand what a single command is doing. I'm sure there is still room for improvement.

Other refactors

I've implemented other small refactors that I considered improved the quality of the code.

TODO/Future work

We should refactor commands (under compile dev) which still use module Commands.Extra.Compile and remove it.

@janmasrovira janmasrovira self-assigned this Mar 25, 2024
@janmasrovira janmasrovira force-pushed the compile-subcommands branch 3 times, most recently from fc44851 to 3305409 Compare March 26, 2024 10:10
@lukaszcz lukaszcz force-pushed the compile-subcommands branch from 3305409 to 785c855 Compare March 26, 2024 16:19
@janmasrovira janmasrovira force-pushed the compile-subcommands branch 3 times, most recently from 40fb107 to 0b66bf4 Compare March 27, 2024 12:08
@janmasrovira janmasrovira added the enhancement New feature or request label Mar 27, 2024
@janmasrovira janmasrovira force-pushed the compile-subcommands branch 7 times, most recently from b12cad9 to 7ba4105 Compare April 3, 2024 11:14
@janmasrovira janmasrovira marked this pull request as ready for review April 4, 2024 07:40
@lukaszcz lukaszcz self-requested a review April 4, 2024 08:02
janmasrovira added a commit to anoma/juvix-docs that referenced this pull request Apr 4, 2024
janmasrovira added a commit to anoma/juvix-docs that referenced this pull request Apr 4, 2024
app/Commands/Dev/DevCompile/Asm.hs Outdated Show resolved Hide resolved
app/Commands/Dev/DevCompile/Tree.hs Outdated Show resolved Hide resolved
app/Commands/Dev/DevCompile/Core.hs Outdated Show resolved Hide resolved
app/Commands/Dev/Core/Compile.hs Show resolved Hide resolved
app/Commands/Dev/DevCompile/Casm.hs Show resolved Hide resolved
src/Juvix/Compiler/Casm/Pretty/Base.hs Outdated Show resolved Hide resolved
@janmasrovira janmasrovira force-pushed the compile-subcommands branch from 652c389 to b1a9594 Compare April 8, 2024 13:05
@janmasrovira janmasrovira requested a review from lukaszcz April 8, 2024 13:07
@lukaszcz lukaszcz force-pushed the compile-subcommands branch from e4f6f38 to 9e6708b Compare April 9, 2024 08:39
@lukaszcz lukaszcz force-pushed the compile-subcommands branch from 9e6708b to 65a69f9 Compare April 9, 2024 09:49
@janmasrovira janmasrovira merged commit 2d36a65 into main Apr 9, 2024
4 checks passed
@janmasrovira janmasrovira deleted the compile-subcommands branch April 9, 2024 11:29
janmasrovira pushed a commit that referenced this pull request Apr 10, 2024
The `--target` flag was replaced by subcommands in
#2700

This PR fixes the benchmark suite to use `juvix compile native` and
`juvix compile wasi`.

In addition this PR adds as `compile-only` target to `juvix-bench`

The compile-only target only compiles the executable for each variant of
each suite. It doesn't actually run the benchmarks. This is useful when
checking that the variant build steps are correct before committing.

Example to run with stack:

```
stack bench --ba 'compile-only'
```
lukaszcz pushed a commit to anoma/juvix-stdlib that referenced this pull request Apr 16, 2024
The `juvix compile` command was changed in
anoma/juvix#2700.

`juvix compile` must be updated to `juvix compile native`.

This PR also updates the test Package lockfile which was not committed
after that last dependency update.
jonaprieto pushed a commit to anoma/juvix-docs that referenced this pull request Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI enhancement New feature or request refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants