-
Notifications
You must be signed in to change notification settings - Fork 86
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
Added ability to trigger on addresses and line numbers #241
Conversation
ba9d12a
to
944ca15
Compare
944ca15
to
5bfa3bb
Compare
core/elf.ml
Outdated
@@ -148,6 +167,72 @@ let find_symbol t name = | |||
None) | |||
;; | |||
|
|||
let find_selection t name : Selection.t option = | |||
let find_line_selection name = | |||
let split_name = String.split name ~on:':' in |
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.
Can we do a
match ... with
| [ desired_filename; desired_line; desired_col ] -> ...
| _ -> raise some error
here?
core/elf.ml
Outdated
| Some desired_filename, Some desired_line, Some filename -> | ||
if String.(desired_filename = filename) && desired_line = line | ||
then cols := (col, state.address) :: !cols | ||
| None, _, _ | _, None, _ | _, _, None -> ()) |
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.
| None, _, _ | _, None, _ | _, _, None -> ()) | |
| _, _, _ -> ()) |
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.
Updated
core/elf.ml
Outdated
then | ||
Core.eprint_s | ||
[%message | ||
"Multiple snapshot symbols on same line. Selecting one." |
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.
Let's switch this to a regular eprintf
with non-sexp formatting because it's user-visible.
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.
Updated
core/elf.ml
Outdated
let find_symbol_selection name = | ||
Option.map (find_symbol t name) ~f:(fun symbol -> Selection.Symbol symbol) | ||
in | ||
if String.is_prefix name ~prefix:"symbol:" |
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.
Let's extract this if/then branch into a short helper and use it here, that takes ~prefix
and ~f
, so that we can elide the hardcoding of the prefix lengths.
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.
Updated
This feature adds triggers of the format `addr:0xfffff` or `line:file.ml:99:15`. All triggers can be prefixed with either `symbol:`, `addr:`, or `line:`, but if none is given, it will default to a symbol if possible and otherwise parse it as a line number or address. There is no feature to allow our fuzzy finding to work with the new options, but it could be possible to implement fuzzy finding on files for example. Signed-off-by: Aaron Lamoreaux <alamoreaux@janestreet.com>
5bfa3bb
to
d8c6318
Compare
Closing since this ended up being merged as part of another PR. |
This feature adds triggers of the format
addr:0xfffff
orline:file.ml:99:15
. All triggers can be prefixed with eithersymbol:
,addr:
, orline:
, but if none is given, it will default to a symbol if possible and otherwise parse it as a line number or address.There is no feature to allow our fuzzy finding to work with the new options, but it could be possible to implement fuzzy finding on files for example.