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

Debugger: add callstack to conditional breakpoints #11273

Merged

Conversation

TryTwo
Copy link
Contributor

@TryTwo TryTwo commented Nov 13, 2022

callstack(0x80123456) returns true if the callstack has an address equal to that value.
callstack("anim") returns true if the callstack contains text equal to the quoted word.
With symbols from address and callstack("80123456"), it will be true if the function that begins at that address is in the callstack. Plus it should return true on anything that the unquoted version returns true for.
!callstack("anim") returns true if the callstack does not contain the text anim.

Had to add string support to expr.h. Strings are flagged by being inside quotations. Strings will return NAN if assigned to a variable or checked for a (double) value.

@TryTwo TryTwo force-pushed the PR_Conditional_BP_Callstack branch 2 times, most recently from 0d015e4 to 505b286 Compare November 13, 2022 14:59
@TryTwo
Copy link
Contributor Author

TryTwo commented Nov 30, 2022

@smurf3tte How does this look to you?

Copy link
Contributor

@smurf3tte smurf3tte left a comment

Choose a reason for hiding this comment

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

The Dolphin side of the change looks fine, but there are some issues with the extension of the expr library. I understand it probably works as-is for well-formed expressions, but it introduces bugs in the lexer and parser that should be taken care of.

Externals/expr/include/expr.h Outdated Show resolved Hide resolved
Externals/expr/include/expr.h Outdated Show resolved Hide resolved
Externals/expr/include/expr.h Outdated Show resolved Hide resolved
Externals/expr/include/expr.h Show resolved Hide resolved
Externals/expr/include/expr.h Outdated Show resolved Hide resolved
Externals/expr/include/expr.h Outdated Show resolved Hide resolved
Externals/expr/include/expr.h Outdated Show resolved Hide resolved
Externals/expr/include/expr.h Outdated Show resolved Hide resolved
Externals/expr/include/expr.h Outdated Show resolved Hide resolved
Externals/expr/include/expr.h Outdated Show resolved Hide resolved
@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 2, 2022

@smurf3tte Thanks so much for the help! I don't actually know any C coding, so I guess it was a bit more haphazard than I thought. I would like to get it correct and I think I fixed most of your issues. Need a bit of help on a few though, as noted.

@TryTwo TryTwo force-pushed the PR_Conditional_BP_Callstack branch from a9ad0b7 to 6ccdbd7 Compare December 3, 2022 01:06
@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 3, 2022

I think I fixed everything mentioned

@smurf3tte
Copy link
Contributor

I don't know what formatting you applied to expr.h, but it wasn't using Dolphin's .clang-format file. Either the formatting should match the rest of Dolphin or it should be left alone, IMO.

Note that Dolphin's .clang-format file is under the Source subdirectory, so it won't be applied to source files under Externals by the lint script or by IDEs like Visual Studio. If you want to apply Dolphin's formatting, the easy thing to do would be to copy the .clang-format file to the expr subdirectory (but don't check it in).

@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 3, 2022

Thanks for pointing out the formatting. I didn't realize it was doing something arbitrary. I just didn't want to format my changes manually , because I'd probably mess something up. I'll fix the things then make a separate push to fix the formatting.

@TryTwo TryTwo force-pushed the PR_Conditional_BP_Callstack branch 2 times, most recently from 55cc48d to 81ece41 Compare December 3, 2022 08:18
@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 3, 2022

I think I returned to the original formatting. It formatted a few lines near the top, but everything else was unchanged.

@TryTwo TryTwo force-pushed the PR_Conditional_BP_Callstack branch from 81ece41 to 4e8f1c8 Compare December 3, 2022 20:23
@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 3, 2022

Made a few more tweaks to try and keep the coding style consistent.

  Use: callstack(0x80000000).
  !callstack(value) works as a 'does not contain'.
Add strings to expr.h conditionals.
  Use quotations: callstack("anim") to check symbols/name.
@TryTwo TryTwo force-pushed the PR_Conditional_BP_Callstack branch from 4e8f1c8 to 76bf1b5 Compare December 4, 2022 03:52
Copy link
Contributor

@smurf3tte smurf3tte 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!

@TryTwo
Copy link
Contributor Author

TryTwo commented Dec 4, 2022

Thanks so much for all the help!!

@AdmiralCurtiss AdmiralCurtiss merged commit 2b93d5e into dolphin-emu:master Dec 4, 2022
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants