symbol names: keep using . on non-Apple unix systems - #14143
Conversation
| # now check the assembly file produced during compilation | ||
| asm=${test_build_directory}/func_sections.s | ||
| grep "\.section \.text\.caml\.camlFunc_sections\\$" "$asm" | wc -l | tr -d ' ' | sed '/^$/d' | ||
| grep "\.section \.text\.caml\.camlFunc_sections." "$asm" | wc -l | tr -d ' ' | sed '/^$/d' |
There was a problem hiding this comment.
Using just . matches any character. I'm not sure this is what you had in mind. Maybe something like that instead?
| grep "\.section \.text\.caml\.camlFunc_sections." "$asm" | wc -l | tr -d ' ' | sed '/^$/d' | |
| grep "\.section \.text\.caml\.camlFunc_sections[.$]" "$asm" | wc -l | tr -d ' ' | sed '/^$/d' |
There was a problem hiding this comment.
That was a quick fix, but yes I agree it is better to narrow this grep.
| * #13050: Use '$' instead of '.' to separate module names in symbol names. | ||
| This changes mangling of OCaml identifiers from | ||
| * #13050, #14104, #?????: Use '$' instead of '.' to separate module names | ||
| in symbol names on Apple and non-unix operating systems. |
There was a problem hiding this comment.
I think being more explicit about which platforms are affected would be a bit nicer:
| in symbol names on Apple and non-unix operating systems. | |
| in symbol names on macOS and Windows (including the Cygwin backend). |
There was a problem hiding this comment.
It is also slightly more correct since with the current code any new operating system will end up using . as a symbol name separator.
xavierleroy
left a comment
xavierleroy
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks! There may be some issues with a couple of tests, see below. Make sure to run a Jenkins CI precheck test.
| (* Return the module separator used when building symbol names. *) | ||
|
|
||
| val escape_prefix: string | ||
| (* Return the escape prefix for hexadecimal escape sequence. *) |
There was a problem hiding this comment.
Perhaps, to be extra clear:
| (* Return the escape prefix for hexadecimal escape sequence. *) | |
| (* Return the escape prefix for hexadecimal escape sequences in symbol names. *) |
|
Precheck is green on the OS/architecture only tested on the Inria side. I am thus planning to merge this afternoon in order to move forward with the beta for OCaml 5.4.0 . |
(cherry picked from commit ffa6197)
4 participants
As discussed in #14104, this PR proposes to revert to using
.as module separator for symbol names on non-Apple unix systems — at the very least for OCaml 5.4.This should restore the support of OCaml symbol name mangling in perf for OCaml 5.4, while keeping the lldb fix on Apple systems . Moreover, it seems likely to me that any tools analyzing symbol names already needs to have OS specific paths, thus we are hopefully not exporting a noticeable amount of complexity to those external tools.
cc @tmcgilchrist