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

Emit bytecode in dune build for debugging #144

Merged
merged 3 commits into from
Aug 6, 2024

Conversation

ahelwer
Copy link
Collaborator

@ahelwer ahelwer commented Jul 14, 2024

This will generate a tlapm.bc OCaml bytecode file in _build/default/src which can be loaded by ocamldebug. Dune 2.0+ requires this to be explicitly specified (docs):

Given an executable stanza with (name <name>), Dune will know how to build <name>.exe. If requested, it will also know how to build <name>.bc and <name>.bc.js (Dune 2.0 and up also need specific configuration (see the modes optional field below)).

ocamldebug also requires everything to be built with the -g flag, which is the case by default with Dune; see output of dune printenv ..

Ref #143

I also added a debug launch profile for VS Code, and added the earlybird DAP server to the opam-deps-dev make target.

@ahelwer
Copy link
Collaborator Author

ahelwer commented Jul 14, 2024

It should be noted that the generated tlapm.bc is very large:

>ls -lh tlapm.*
-r-xr-xr-x 1 ahelwer ahelwer  22M Jul 14 18:30 tlapm.bc
-r-xr-xr-x 1 ahelwer ahelwer 5.1M Jul 14 18:30 tlapm.exe
-r--r--r-- 1 ahelwer ahelwer  178 Jul 14 17:55 tlapm.ml
-r--r--r-- 1 ahelwer ahelwer  139 Jul 14 17:55 tlapm.mli

So perhaps these changes must be paired with logic to avoid bundling it into the zip file. Although looking at the make release target perhaps this is not actually an issue and the .bc file will not be included.

@ahelwer ahelwer force-pushed the emit-bytecode branch 5 times, most recently from 9b45372 to 457dd74 Compare July 15, 2024 00:12
@ahelwer ahelwer marked this pull request as draft July 15, 2024 00:14
@ahelwer ahelwer marked this pull request as ready for review July 28, 2024 12:01
@ahelwer
Copy link
Collaborator Author

ahelwer commented Aug 5, 2024

@kape1395 what do you think of these changes?

Copy link
Collaborator

@kape1395 kape1395 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
As I see, the bc file is not in the release.

@kape1395 kape1395 merged commit 7a27933 into tlaplus:main Aug 6, 2024
5 checks passed
@ahelwer
Copy link
Collaborator Author

ahelwer commented Aug 9, 2024

@kape1395 actually I think these changes should be reverted, because after a few weeks playing around with the OCaml debugger I have found it is just too immature to use as a development tool and the utop REPL should be used instead. This seems to be the general idea in the OCaml community as well.

@kape1395
Copy link
Collaborator

@ahelwer, I would like to keep the changes. Even if they are not that useful today, they don't harm anyone either.

@ahelwer
Copy link
Collaborator Author

ahelwer commented Aug 16, 2024

All right, although in order to be able to actually set breakpoints (in some places) you need this dune parameter set to false (it's true by default), and I don't know whether we want that: https://dune.readthedocs.io/en/stable/reference/dune-project/map_workspace_root.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants