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

CN debugger: allow debugging of CN via Gillian #144

Open
wants to merge 24 commits into
base: sc/cn-debug
Choose a base branch
from

Conversation

kiranandcode
Copy link
Collaborator

This PR builds on #128 and ports it to work with the main branch of VERSE-toolchain.

@samcowger
Copy link
Contributor

Unless you object, I'd like to base this PR on sc/cn-debug instead of main. It looks like it squashes that branch, as well as whatever changes you've made to it, into a single commit, which makes it difficult to see what work you're hoping to merge in.

@kiranandcode
Copy link
Collaborator Author

Oh, yep, that makes sense.

@samcowger samcowger changed the base branch from main to sc/cn-debug January 22, 2025 16:30
@kiranandcode
Copy link
Collaborator Author

I rebased cn-debug onto main, and then added my changes in my branch on top of them @samcowger

@kiranandcode
Copy link
Collaborator Author

Added functionality to generate and dump state traces + filter lenses by whether the json files have been generated or not

@samcowger
Copy link
Contributor

My apologies, I keep changing main out from under you! I'm going to rebase sc/cn-debug onto main myself, because there are some changes I'd like to make to organize it as a sibling dune project to cn-lsp and telemetry. I think that should make this PR smaller and easier to understand, and will help us focus on the functional changes you've made.

@samcowger samcowger force-pushed the sc/cn-debug branch 2 times, most recently from e63ab53 to e9ebbca Compare January 24, 2025 18:39
@samcowger
Copy link
Contributor

I am surprised by how poorly GitHub seems to be handling this - I see that you've made changes to cn-lsp/lib/lenses.ml in 846377f, for example, so why on earth should the PR's diff show that as a completely new file?

At any rate - I forgot that cn-dap needs to build on OCaml 5.1.1. which throws a bit of a wrench into development and CI. I've been trying to maintain compatibility with 4.14.1, because that's what CN officially supports. In practice, though, I think it works fine on 5.1.1 and people use that to develop on it, so I'm going to open a PR to upgrade this repo's tools to 5.1.1 in support of getting this debugger merged in.

@kiranandcode
Copy link
Collaborator Author

oh, yep, sounds good, let me know once cn-debug is in a stable state and I'll rebase my changes onto it, and hopefully should produce a more coherent diff.

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

Successfully merging this pull request may close these issues.

2 participants