-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add support for data breakpoints #16505
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
base: master
Are you sure you want to change the base?
Conversation
|
@colin-grant-work, many thanks for implementing this feature -- great to see data breakpoints coming to Theia! I tested the PR locally by checking out the branch in my Theia repo clone and building the Theia Electron app. In Theia, I do see the three context menu entries (Break When Value Changes / Is Read / Is Accessed) on variables (see screenshot), but they don’t seem to have any effect -- no data breakpoint appears in the Breakpoints view, and execution never stops as expected. Also, I don’t see the “Add Data Breakpoint at Address” command in the Command Palette, nor the new toolbar item in the Breakpoints view. Let me know if there are any additional steps or configuration needed to enable these features. |
|
PS: I have added this line at the beginning of console.log("~~~ currentSession?.capabilities", currentSession?.capabilities);It gave me this console output: This seems to confirm that data breakpoints are definitely supported by cdt-gdb-adapter. |
| } | ||
|
|
||
| getDataBreakpoints(): DebugDataBreakpoint[] { | ||
| if (this.capabilities.supportsInstructionBreakpoints) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this check for this.capabilities.supportsDataBreakpoints?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should indeed - bad copy paste.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This likely explains why data breakpoints weren't being rendered for you: looks like I was testing in a case where both instruction breakpoints and data breakpoints were supported, so this mismatch didn't affect the rendering in my case, but in yours, instruction breakpoints weren't supported, so this method was behaving incorrectly.
@chroberino, with this set of capabilities, you should not see that command. It depends on the On the other hand, it looks like that's supposed to be treated as defaulting to true, so I'll switch that logic around. |
|
@colin-grant-work, I have just tested with your latest changes. I can set data breakpoints, now! What I also noticed: I guess it is not intended to allow users adding data breakpoints also via the "Debug Console" window? |
|
Thanks again, @colin-grant-work, I’ve tested with your latest change, and the misplaced data breakpoint icon in the "Debug Console" window is gone now.
After some investigation, I found this breakpoint hitting issue isn’t specific to Theia, it behaves the same in VS Code as well (=> eclipse-cdt-cloud/cdt-gdb-adapter#458 (comment)) I’ve noticed one remaining point: |
Hm... I suspect that this is another case where I'm supposed to treat something |
|
@colin-grant-work, I have just noticed that you have already worked on persisting data breakpoints. 🙏 I did notice the following issue with that: I’ve recorded a screen capture demonstrating the behavior: |
|
@colin-grant-work, great news! I finally got everything working! |




What it does
Closes #16445
This PR hooks up the DAP's data breakpoint requests to Theia's debug system.
How to test
Note
As noted in #16445, the CDT-GDB Adapter supports data breakpoints. In my testing, I used Rust with Code LLDB
Follow-ups
Breaking changes
Attribution
Review checklist
Reminder for reviewers