Page MenuHomePhabricator

[OCaml] Add OCaml APIs to access DebugInfo
Needs ReviewPublic

Authored by jberdine on Apr 19 2019, 8:06 AM.

Details

Summary

This diff adds a debuginfo sublibrary to the ocaml bindings
library. This is used to expose the DebugInfo API, mostly as declared
in include/llvm-c/DebugInfo.h. Currently this is incomplete: only
containing the functions needed to obtain the debug location
directory, filename, line, and column of instructions, global
variables, and functions.

Diff Detail

Event Timeline

jberdine created this revision.Apr 19 2019, 8:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2019, 8:06 AM

This is an alternative to D52239 which adds OCaml wrappers of the C API that are as thin as I see how to make them. I added a new sublibrary since the exposed functions come from DebugInfo.h instead of Core.h, and that seemed to be the pattern for the other sublibraries. Does this seem like the right approach?

To test this I have an OCaml client (a slightly modified version of https://github.com/facebook/infer/blob/master/sledge/src/llair/frontend.ml) that reads and prints the debug info using the functions in llvm_debuginfo.mli for all of the .bc and .ll files under the test directory, as well as a number of larger internal compilation units.

@whitequark, with this diff's approach of creating a hierarchy of types to mirror the LLVM-C DI types, is it acceptable to add the types and functions incrementally? That is, could we land this diff and add other types and functions later?

Another question is about opam. Since this diff adds a sub-library, the opam package files will need to change. Is there any experience / best practice about how to handle this? In particular, since the opam package files are not in the llvm repo, it will not be easily possible to pin the llvm dev repo.

@whitequark, with this diff's approach of creating a hierarchy of types to mirror the LLVM-C DI types, is it acceptable to add the types and functions incrementally? That is, could we land this diff and add other types and functions later?

Yes, that would be perfectly fine, and is in line with the extension of LLVM-C in the past.

Another question is about opam. Since this diff adds a sub-library, the opam package files will need to change. Is there any experience / best practice about how to handle this? In particular, since the opam package files are not in the llvm repo, it will not be easily possible to pin the llvm dev repo.

I'm afraid there's no existing procedure for this. If the opam file does not have to be in the root of the repo (I forget exactly how it works), we could add it here.

I will experiment with putting the opam file below the repo root. Another potential stumbling point is that the opam package includes some patches for the build/install, see https://github.com/ocaml/opam-repository/tree/master/packages/llvm/llvm.9.0.0/files . Would it be too strange to include such patches here?

Would it be too strange to include such patches here?

Yes. The patches should either be removed or investigated and applied in tree.