Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005899OCamlstandard librarypublic2013-01-22 15:192015-12-11 19:26
Assigned Tojacques-henri.jourdan 
PrioritynormalSeverityfeatureReproducibilityhave not tried
PlatformOSOS Version
Product Version 
Target Version4.02.0+devFixed in Version 
Summary0005899: Expose a way to inspect the current call stack
DescriptionThe machinery required to support exception traces actually makes it possible to return, at a given point in the program, (a fragment of) the call stack of the current context. This can be useful on its own. I can think of the following uses:

  - debug;

  - log the context of an error, where it is detected, but without failing;

  - run the code with exception traces disabled, for performance reasons, but manually extract the stack and put it as an argument of some specific exception, when it is raised;

  - instrument the code to perform context-sensitive profiling (how many times is this function called and from which call sites).

I attach a patch which concretely reuses the mechanism for exception traces. With more effort, it would be possible to do something cleaner. And there is no reason to put the new feature in Printexc, since it is not related to exceptions at all.
Attached Files? file icon patch [^] (24,155 bytes) 2014-05-06 10:43 [Show Content]

- Relationships
related to 0005935closedfrisch A faster version of "raise" which does not maintain the backtrace 
related to 0006302closedjacques-henri.jourdan Very slow backtraces in bytecode : debug infos read from the filesystem everytime 

-  Notes
dbuenzli (reporter)
2013-01-22 15:48

A little bit OT but could we at the same time have the same resulting type as Printexc.current_call_stack for a variant of Printexc.get_backtrace ? I already had to parse this string more than once (notably to be able to pretty print it). Thanks.
frisch (developer)
2013-01-22 15:48

Daniel: agreed!
shinwell (developer)
2013-01-22 15:54

We already have something similar in Core (see module [Backtrace]). I was planning to clean this up; maybe we could merge these patches.
frisch (developer)
2013-01-22 16:01

Mark: great! Let's wait to see if there is any theoretical opposition against the proposal from other core developers. I really believe the feature deserves to be in the core system.
dim (developer)
2013-01-22 16:47

I tried that too for Lwt once ;-) But it wasn't enough so I didn't proposed it.

I'm also for having this.
jacques-henri.jourdan (manager)
2013-04-16 11:14

What I propose is the following :

1- Two functions set_backtrace_limit : option int -> unit and get_backtrace_limit : unit -> option int. set_backtrace_limit changes the mode of the backtrace recorder.
  * If called with None, we have the current behavior
  * If called with Some l, the l last stack cells will be recorded, even if they are beyond the exception handler.

This allows the client code to easily implement the current_call_stack function itself, while allowing to record a full backtrace when an exception is raised for debugging purposes.

2- Exposing the backtrace internal type of the Printexc module, and the function convert_raw_backtrace : raw_backtrace -> backtrace option, in order to make the client code able to manipulate the backtrace programatically.

I am not really in favor of 2), because what is recorded in the backtrace is not well specified. It has subtle interaction with the "-g" flag, with whether the code is executed in bytecode mode or in native mode...
gasche (developer)
2013-04-16 11:54
edited on: 2013-04-16 12:00

So the way to implement current_call_stack would be to raise an exception and immediately catch it back, with a longer backtrace limit?

I think that's a bit ugly, and besides you have no way to tell in your interface that you want the trace upto the beginning of the application (sure you can put max_int there, but urgh.).

On the other hand, if you had a feature to just return the N last cells (or all upto the start if None is passed), you could also express the behavior of recording "beyond the exception handler" in user-code: just use this new feature in the "with" block, that by construction will see the same cell history as the "raise" it catched (I mean "get 10 more cells higher than my try" at raise point is equivalent to "get 10 cells" at the handler point).

So while I agree that "get farther than my raise" and "get me some cells" are equivalently expressive, I think the latter is a less convoluted primitive to start with.

frisch (developer)
2013-04-16 13:18

> On the other hand, if you had a feature to just return the N last cells (or all upto the start if None is passed)

I agree that this is indeed more user-friendly.
gasche (developer)
2013-04-16 14:36

Another point about the different interface: it allows to have the number of desired cell in the function call, instead of as a global mutable variable (which was necessary with the previous model, as "raise" cannot be parametrized).
jacques-henri.jourdan (manager)
2013-04-16 15:54

I understand that not using a global variable seems to be a good thing. However, for debugging, using a global variable is convenient, as it does not imply modifying the handler code.

Concerning the fact that there is no way to ask for the whole trace, I am in favor of that : the user has to know that putting a big value here could be a performance problem in case of a big backtrace.
frisch (developer)
2013-04-16 16:38

> However, for debugging, using a global variable is convenient, as it does not imply modifying the handler code.

I don't known what kind of debugging you have in mind, but for many uses cases of the proposed feature ("get a description of the top N slots of the current call stack"), there won't even be any exception at all in the client code.
jacques-henri.jourdan (manager)
2013-04-16 17:18

> there won't even be any exception at all in the client code.

This is not a real problem: you can create one as in the proposed patch. I agree that for the use cases of the proposed feature, the problem does not change.

However, I see an other use case : what I have in mind is that sometimes it is not enough to see the backtrace of an exception, and having some context on where it has been catched is usefull. Both approaches can help. It can be achieved with the global variable approach by modifying the global variable, and in the gasche's approach by modifying the handler's code. My point is that while debugging, modifying a global variable is maybe more convenient than modifying a handler : there might be several interesting handlers, you have to keep track where they are...

By the way, I am thinking of an other feature that could be interesting : why not adding a record_and_raise primitive of the Printexc module, that raises an exception, recording its backtrace even if the backtrace flag is deactivated. That could be interesting, for example, when a performance critical code raises an exception signaling an error, while this code uses exceptions for control (so that you do not want to activate backtraces globally). What do you think ?
frisch (developer)
2013-04-16 17:32

> This is not a real problem: you can create one as in the proposed patch.

I'm not against the global variables (in Printexc) if you think it can simplify debugging existing code, but the standard library should also expose a nice API (maybe not in Printexc) to retrieve with top of the stack without a hack (changing a global value, raising a dummy exception, filtering the result, restoring the global value).

> why not adding a record_and_raise primitive of the Printexc module, that raises an exception, recording its backtrace even if the backtrace flag is deactivated.

I don't think you can easily traverse intermediate exception handlers which re-raise the exception.

> for example, when a performance critical code raises an exception signaling an error

I don't think that in practice people will segregate their code into performance-critical modules (compiled without -g) and others (compiled with -g). Tweaking compilation flags on a file-by-file basis is not realistic for large code bases. The safe assumption is rather that code running production will be entirely compiled with -g and executed with stacktrace enabled, and then it makes more sense to provide a light "raise" for those cases where performance matters.
jacques-henri.jourdan (manager)
2013-04-21 17:01

I uploaded a patch to implement the feature. It brings the following advantages from the original one :

- It does not use an exception in the standard library code. That is, it does not erase the exception backtrace buffer.
- It is possible to give a maximum number of frames to record.

It is still needed to provide a way to programmatically inspect raw_bactrace inhabitants. On this question, I do not know what is the best thing to do : we can either make public the "backtrace" private type of the Printexc module, or provide an interface in the style of the alain's patch.
gasche (developer)
2013-04-21 19:07
edited on: 2013-04-21 19:13

Call me superstitious, but I would feel better without the name change and corresponding asm-level renames. What about just keeping ..._backtrace and adding ..._backtrace_custom?

Regarding programmatic inspection, I think that the "backtrace" type inside the Printexc module should remain abstract (to allow for easy change of implementation later), with a conversion function (that may be just the identity) to a public type, that may be either the current type or Alain's simpler interface. Of course, so far we aren't exposing any of the functions manipulating this middle-end type, so the distinction between this and a fully public type does not exist yet.

jacques-henri.jourdan (manager)
2013-04-22 12:14

New version : keeping the names of the ASM functions, added list_of_raw_backtrace.
frisch (developer)
2013-04-22 12:17

I believe the new [current_backtrace] function should rather be in Pervasives since it is not related to exceptions.
gasche (developer)
2013-04-22 13:49

I'm worried about the (<> "") hack coming from Alain's original patch. What is that supposed to do (it would deserve a comment in the code at least), and won't it blow in our face in some scenarios? (You really want to trust the backtrace-reporting functions.)

Other than that, I think the documentation for current_backtrace should mention the fact that it will collecth the [n] last frames, or less if less are available. I understand the idea of not favoring the "get me all traces" scenario (though frankly I'm still not fond of having to use a high constant in this case), but the documentation should at least make clear that it is possible and won't fail instead.

Alain > moving current_backtrace to Pervasives may require a bit of work, as it currently depends an the abstract type defined in Printexc, and we can't have Pervasives depending on Printexc. Before Jacques-Henri gently does the work, are you sure this is desirable, and do you have more precise guidelines on what you want (eg. should we have raw_backtrace defined in Pervasives and re-exported in Printexc? Only in pervasives? What about an eventual exposed 'backtrace' type? Should 'record_backtrace' also move?). My gut feeling is that it's a bit perfectionist.
frisch (developer)
2013-04-22 14:05

> What is that supposed to do

In the original form, it was supposed to remove the top stack slots corresponding to the fake exception handler used to trigger the stack capture. (IIRC, we needed to drop 1 or 2 slots depending on whether we were running bytecode vs. native).

> are you sure this is desirable,

Desirable, yes, important no.

> we have raw_backtrace defined in Pervasives and re-exported in Printexc

If the two features share the same type, then yes, raw_backtrace could go to Pervasives. I don't think it is then required to re-export it from Printexc.

Or maybe we could introduce a new

> What about an eventual exposed 'backtrace' type

Same as raw_backtrace, I guess.

> Should 'record_backtrace' also move

This is related to exceptions (even if not to printing them), so Printexc seems fine.

An alternative could be to introduce a new stdlib module "Backtrace", where everything related to stack traces (even for exceptions) would go. Printexc would only keep the exception-printing functions (plus the one which exist in 4.00, marked as deprecated, for compatibility). But this might be overkill for a small feature, hence my suggestion to move the new things to Pervasives. But you should definitely check with Xavier!
jacques-henri.jourdan (manager)
2013-04-22 14:21

> In the original form, it was supposed to remove the top stack slots corresponding to the fake exception handler used to trigger the stack capture. (IIRC, we needed to drop 1 or 2 slots depending on whether we were running bytecode vs. native).

I removed it. It is no longer necessary in this context.

Concerning Printexc vs. Pervasives vs. Backtrace, I would be definitely in favor of putting it in a new Backtrace module, and this is not very difficult. But this is a rather more philosophical question.

There is another perfectionists-kind problem : now, the printer of backtraces is shared between exception backtraces and this new kind of backtraces. This causes the first line of the backtrace to be :

"Raised by primitive operation at ...."

instead of

"Called at ..."

I do not know what kind of "primitive operation" can cause this message appear in the case of an exception (array accesses ? match failures ? static raises ? ... ?), but I wonder whether it is possible to "re-raise by primitive operation", in which case the message won't be displayed correctly anyway...
gasche (developer)
2013-04-22 15:23

Xavier remarks that a Backtrace module would risk conflicting with userspace modules of the same name (that exist in some dark place with probability 1).

Alain > none of Jacques-Henri's recent raw_backtrace work has been released yet; as far as I know, there was no change in Printexc since 3.12, so we have no deprecation to worry about: the raw_backtrace stuff can be moved, and even renamed, if deemed necessary.

After some more reflection, I find it weird that list_of_raw_backtrace may lose information if called on a trace recorded from an exception: as the re-raise information is ignored, it is only an injection if it is only used on traces produced by current_backtrace. I think we should either have two types (error_trace and call_trace) and two printing functions, or expose a representation that can fully handle both situations (eg. the one currently used in printexc). A conceptual test for any API design would be to check that the printing function(s) can be reimplemented from the user side, using only the exposed program-friendly representation.
frisch (developer)
2013-04-23 11:52

> none of Jacques-Henri's recent raw_backtrace work has been released yet; as far as I know, there was no change in Printexc since 3.12, so we have no deprecation to worry about

I know, but there was already some backtrace-oriented functions in Printexc in 4.00 (like record_backtrace).

> Xavier remarks that a Backtrace module would risk conflicting with userspace modules of the same name (that exist in some dark place with probability 1).

Ah, I guess we desperately need namespaces, then. We could use OCaml_backtrace for now and hope that this namespace system will allow it to be re-mapped to just "Backtrace" in client code :-) Or just put everything in Pervasives.

Note that technically, any addition to the stdlib could break existing code (with probability < 1, I agree).
jacques-henri.jourdan (manager)
2013-05-27 11:20

So, what is the conclusion of this discussion ?

Should I create a new module of the stdlib, called OCaml_backtrace, that contains all the functions related to backtraces, and depreciate existing ones ?

I would rather not adding this to Pervasives, as it is expected to be used only by some specialized parts of code.
frisch (developer)
2013-06-03 22:37

> So, what is the conclusion of this discussion ?

I don't know. After all, keeping everything in Printexc -- even if illogical -- might not be so bad.
gasche (developer)
2013-06-04 07:56

I agree that Printexc is a good-enough choice. One more reason is that the security-minded people (eg. LaFoSec study) will want to disable this function as well as the others in Printexc.
gasche (developer)
2013-06-13 10:17

Progress note: I'm currently reviewing the patch and changing/discussing it with Jacques-Henri.
gasche (developer)
2013-06-15 23:28

We came up with a new patch proposal that is eager to get external reviews.

(1) patch caml_stash_current_backtrace.diff implements the runtime
primitive nece ssary for returning the current backtrace. Besides
fixing a couple issues in Jacques-Henri previous patch, we now use
a different implementation strategy: instead of using a dynamically
reallocated buffer, we first traverse the stack to compute its trace
then allocate once with the right size. The behavior of
caml_stash_backtrace, computing the backtrace of an exception, is
unchanged from the trunk implementation.

(2) patch printexc_get_callstack.diff implements the user-visible
layer in and printexc.mli. I think there is room for
discussion for what interface is desirable (including poignant
variable naming choices). The previous patch exposed the following

 val current_backtrace: int -> raw_backtrace
 (** [Printexc.current_backtrace n] returns the backtrace of the current
     stack. The backtrace is truncated to the highest [n] frames. *)

 val list_of_raw_backtrace: raw_backtrace -> (string * int * int * int) list
  (** [Printexc.list_of_raw_backtrace bt] converts a raw backtrace to a
      list of locations. Each item is a tuple (file name, line number,
      first character, last character). *)

the current patch instead exposes the following interface:

  val get_callstack: int -> raw_backtrace
  (** [Printexc.get_raw_callstack n] returns the backtrace corresponding
      to the current call stack -- as opposed to the backtrace of the
      exception raised last. The backtrace is truncated to the
      highest [n] frames. *)

  type loc_info =
    | Known_location of bool (* is_raise *)
                      * string (* filename *)
                      * int (* line number *)
                      * int (* start char *)
                      * int (* end char *)
    | Unknown_location of bool (*is_raise*)
  (** The [loc_info] type represents the location information present in
      each slot of the backtrace. *)
  val loc_info_of_raw_backtrace : raw_backtrace -> loc_info array option
  (** Extracts the location information of a backtrace value. The
      location of the most recent program point (the one corresponding
      to the [raise] or call to [current_backtrace]) is at index [0].
  val loc_info_to_string : int -> loc_info -> string
  (** Prints location information, in a way compatible with the
      behaviour of [backtrace_to_string]. The integer parameter is meant
      to be a position in a trace, as the most recent location
      (index [0]) is printed in a specific way.

The type "loc_info" is precisely the one used in the Printexc
implementation (but note that it would be possible to change the
implementation and still expose this interface to the user, with one
conversion step). This gives the end-user more flexibility, which
I suspected Alain in particular may appreciate, but I'm not sure
whether that's actually needed.

Note that for both interfaces, the user is forced to give a maximal
length when requesting the current call trace. Jacques-Henri suggests
that this interface makes the user fully aware of performance
implications -- as opposed, for example, to giving an [int option],
with [None] meaning "until the end".

In the case of both (1) and (2), I would also welcome feedback on the
naming choices. In the latter patch I picked [get_callstack] rather
than [current_backtrace] (the idea being to use [callstack] rather
than [backtrace] to differentiate whether or not it is tied to
an exception), but many choices are possible and I'm interested to
constructive arguments. For the runtime primitive the naming is less
important, but I may change it to something more in line with the
exposed name (eg. [caml_get_callstack] if we agree on [callstack]).
dbuenzli (reporter)
2013-06-16 00:16


1) Why not simply `loc` instead of `loc_info` ?

2) If 1) is adopted I'd suggest `locs_of_raw_backtrace` (note the plural) and `loc_to_string`

3) Why the `_location` suffix in the variant ? It seems redundant and longwinded. In practice your likely to write something like `match loc with Known _ -> ... | Unknown _ -> ...`.
gasche (developer)
2013-06-16 11:33

That is sound advice, thanks.

However, the naming of loc_info was not of my choosing: contrarily to the rest of the exposed interface that is new, this type is what was used internally in the Printexc implementation. I'll have to check that it is ok to rename it (may make life harder for people with on-top-of-the-compiler patches, etc.), and maybe decide not to change it after all.

You have probably noticed that loc_info_of_raw_backtrace is partial (may fail when no debug information is available); since I submitted the patch here, I added an explicit mention of it in the .mli documentation.
frisch (developer)
2013-06-17 09:43
edited on: 2013-06-17 09:45

> type loc_info = ...

I'd propose something more self-documented and more extensible. What about:

type loc_info =
    is_raise: bool;
    call_site: loc option;

and loc =
    filename: string;
    line: int;
    start_char: int;
    end_char: int;


jacques-henri.jourdan (manager)
2013-06-17 16:23

gasche: Thank you for refactoring my patch ! It seems to be pretty good work.

dbuenzli: frisch: While I agree there is some room for improvement in the naming, it would be desirable to keep the same behavior for the caml_stash_current_backtrace external. In particular, changing memory layout for the location information is not a good idea, I think. dbuenzli ideas seem OK.
gasche (developer)
2013-06-19 16:58

The C primitive part is now integrated into OCaml (trunk and 4.01).

Alain, could you clarify whether:

1. you're fine with a linear translation pass between the loc_info structure to a record that is more comfortable to manipulate

2. or you favor performance over usability and would rather keep the primitive data structure

3. you absolutely need both and I have to consider writing C stubs that directly go from the raw stackframe to the nice record structure without the existing intermediary OCaml datastructure (this would still be a less memory-efficient layout than the current choice, but I don't think it matters in practice)

Of course I'd rather avoid option (3).
frisch (developer)
2013-06-19 18:59

(3) would seem the best solution to me. I'm undecided between (1) and (2). I can imagine cases where performance would matter a little bit (e.g. an instrumentation tool capturing the top of the call stack when entering any function, to extract a call graph), but I'm not sure it's worth exposing a less friendly interface.
jacques-henri.jourdan (manager)
2013-06-19 19:12

I would say that (3) has a major drawback: it will break the backward compatibility of the C stub, and this won't be spotted by any type checker...
frisch (developer)
2013-06-19 19:44

> it will break the backward compatibility of the C stub, and this won't be spotted by any type checker...

I don't think that the runtime representation is documented. Which code would be affected?
gasche (developer)
2013-06-19 22:29

I wasn't thinking of changing any of the existing C stubs. We love C stubs so much that there is always room for more.
frisch (developer)
2013-07-08 12:12

What about exposing:

 external get_callstack: int -> raw_backtrace = "caml_get_current_callstack"

in Printexc for 4.01? With the existing raw_backtrace_to_string function, it is already very useful, and this API is unlikely to change.
gasche (developer)
2013-07-10 19:18

My hope was to get the time to develop a more complete solution, but I haven't get to do it yet (and will be away for holidays for most of the time between now and beginning of August). If you or Jacques-Henri wants to tackle something, that would be fine. Otherwise exposing this function is already a good first step for 4.01, and we can discuss additional features for trunk later.

I think a nice interface would have been along the lines of:

  type call_kind = Raise | Call
  type location = { ... }

  type backtrace = (call_kind * location) array
  type callstack = location array

with a C-side variant of caml_convert_raw_backtrace that would return a "callstack" directly (I looked, and most of the code would actually be shared in the extract_loc_info function, so that can be implemented rather painlessly).
frisch (developer)
2013-07-10 19:42

Ok, I'll just expose get_callstack in Printexc (so that it is available in 4.01). This is much better than nothing.
frisch (developer)
2013-07-11 14:37

Printexc.get_callstack is now available (commit 13886 on trunk, 13884/13885 on 4.01).
dbuenzli (reporter)
2014-01-29 14:31

Just noting that I'm currently parsing the result of raw_backtrace_to_string and while I thank for having the get_callstack function, I'm slightly annoyed.
frisch (developer)
2014-02-28 10:26

It will also be useful to implement generic operation (hash, compare) for the raw_backtrace datatype. An interesting use of get_callstack is to instrument code to collect statistics such as the "most common" call sites for a given function; being able to compare opaque raw_backtrace would make this quite efficient.
jacques-henri.jourdan (manager)
2014-03-21 16:17

I come with a new patch to include those remarks. There are several changes:

- raw_backtrace is no longer an abstract type, but rather an raw_backtrace_slot array, where raw_backtrace_slot is an abstract type. raw_backtrace_slot elements are hashable and comparable. At runtime, values of this type contain either a bytecode pointer or a frame_descr pointer. In order to prevent the GC from walking through this pointer, the low-order bit is set to 1 when stored in the array.

- I made the old loc_info type public, renaming it to backtrace_slot

- I added a new primitive :

val convert_raw_backtrace_slot: raw_backtrace_slot -> backtrace_slot

Rather than returning an option, it raises Failure when it is not possible to get the debugging information. It seems more idiomatic, especially because the exceptional case cannot appear only for a part of the executable.

- I removed the caml_convert_raw_backtrace primitive, that was deprecated in the lattest version, and which is more difficult to implement in the C side because of the new exception interface described above.

- In the bytecode runtime, the events are no longer deserialized once for each conversion, but once and for all at the first conversion, and stored in a global variable. I believe this information should not take so much memory in practice (it uses the same order of magnitude memory as the bytecode executable).
dbuenzli (reporter)
2014-03-21 16:30

I'm not sure I understand the rationale behind having both raw_backtrace_slot and backtrace_slot and exposing the structure of raw_backtraces. I'd rather have only backtrace_slot and a function:

slots_of_raw_backtrace : raw_backtrace -> backtrace_slot list
gasche (developer)
2014-03-21 17:32
edited on: 2014-03-21 17:32

The design is constrained by many different use cases.

Jacques-Henri uses raw slots for statistical profiling; during profiling, you do not need to inspect OCaml-friendly information, just to store traces away as fast as possible -- and turn them into non-raw slots or string only later when presenting results. Having access to slots instead of just the raw trace is useful to share common suffixes of different traces.

You're interested in programmer-friendly access to trace locations. The current design makes this possible, but maybe not as friendly as one could hope (eg. get a record with well-named fields, as in asmcomp/debuginfo.mli [^] ), to avoid an additional data conversion pass -- or making the change more invasive.

dbuenzli (reporter)
2014-03-21 17:53

Ok didn't get this was being used for other things. No need for a record for me, the access is sufficiently programmer-friendly through pattern matching, especially if this means nice profiling tools in the future... I was just concerned that exposing the raw_backtrace structure was a needless interface commitment.
frisch (developer)
2014-03-24 15:40

I did not look at the implementation, but I'm ok with the proposed API.
frisch (developer)
2014-03-28 10:49

Concerning the part which addresses 0006302 (not reloading debug events every time in bytecode), I'm wondering if it wouldn't be worth copying the data into a native C data structure. It's not the RAM usage which worries me, but the fact that we keep a rather big data structure in the OCaml heap (which means some overhead for every major GC).
jacques-henri.jourdan (manager)
2014-04-04 16:50

Yet another patch, addressing Alain's comment.

It is also more efficient, as the events are stored in ram in increasing pc order, so that a binary search can be performed when converting backtrace slots.
gasche (developer)
2014-04-08 15:25

I just posted a github PR with a few additional changes on top of JH's patch, and a bootstrapped compiler: [^]
frisch (developer)
2014-05-05 15:23

This feature is quite useful, and it would be sad to miss the 4.02 feature freeze because of the API design. I suggest to include the latest patch and mark the API as experimental in the documentation.
jacques-henri.jourdan (manager)
2014-05-05 15:40

I attach a new version of the patch : it removes the backtrace_slot type, and provide getters and setters over the raw_backtrace type, so that the payload is extensible.

I do not expect the conversion to take a long time, as it is mainly a context switch to C code, some bits twiddling and a small allocation.

I also documented some functions left undocumented in the previous release.

Does the API convince you ?
frisch (developer)
2014-05-05 15:51

I'm ok with this latest API. It will always be possible to add in the future a function which returns all the relevant information together if performance concerns justify it.

I'm in favor of pushing the patch to trunk (you might want to wait a few hours to see if someone on caml-devel objects to it).
dbuenzli (reporter)
2014-05-05 16:05

I personally dislike the use of `Not_found` in the accessors, but OTOH having an option for each of the cases is painful when you know they are all there. To avoid that either a location type (which could also be abstract with accessor) could be introduced (e.g. see the proposal on github by gasche) and you return an option on that location type or we should have:

1. Raw_backtrace_slot.has_location : t -> bool
2. The documented guarantee that if Raw_backtrace.slot t is true, none of the accessors will raise Not_found.
jacques-henri.jourdan (manager)
2014-05-06 10:43

I added has_location.
dbuenzli (reporter)
2014-05-06 11:03

Thanks ! Another thing. Is there any boolean to check to detect if Failure will be raised ? If that is the case the functions could raise Invalid_argument whenever that boolean is set to a given value, rather than Failure (that's a rather personal point of view, use only the Invalid_argument exception for denoting programming errors, here the error being calling these function when the flag is set to a given value). This boolean could also be used at early program startup to indicate that locations will not be reported rather than having to try it to discover that this will not be the case.
jacques-henri.jourdan (manager)
2014-05-06 11:31

Basically, Failure is raised when it has not been possible to read the debugging information from a bytecode executable. This can happen for several reasons (not linked with -g, executable file not found, out of memory...).

However, this cannot happen when this info has been loaded. That is, if one of these calls succeed, none will fail in the future. I can provide a function to load debugging information before using it, guaranteeing it will not raise Failure in the future.

We could choose not to report this error and raise Not_found (or even Invalid_argument) instead, but the error message gives some information about the actual cause of the error.

I do not know what is the best thing to do...
dbuenzli (reporter)
2014-05-06 12:08

Thought a little bit about alternatives, but maybe it's alright the way it is now then.
gasche (developer)
2014-05-10 21:41

Merged in trunk, with a small variant of Jacques-Henri's API (no more exceptions, the raw_backtrace type is kept private).

- Issue History
Date Modified Username Field Change
2013-01-22 15:19 frisch New Issue
2013-01-22 15:19 frisch File Added: current_call_stack.diff
2013-01-22 15:21 frisch Description Updated View Revisions
2013-01-22 15:48 dbuenzli Note Added: 0008779
2013-01-22 15:48 frisch Note Added: 0008780
2013-01-22 15:54 shinwell Note Added: 0008781
2013-01-22 16:01 frisch Note Added: 0008782
2013-01-22 16:47 dim Note Added: 0008783
2013-01-23 09:42 frisch Severity minor => feature
2013-04-15 11:31 frisch Assigned To => jacques-henri.jourdan
2013-04-15 11:31 frisch Status new => assigned
2013-04-16 11:14 jacques-henri.jourdan Note Added: 0009129
2013-04-16 11:54 gasche Note Added: 0009131
2013-04-16 12:00 gasche Note Edited: 0009131 View Revisions
2013-04-16 13:18 frisch Note Added: 0009137
2013-04-16 14:36 gasche Note Added: 0009140
2013-04-16 15:54 jacques-henri.jourdan Note Added: 0009146
2013-04-16 16:38 frisch Note Added: 0009147
2013-04-16 17:18 jacques-henri.jourdan Note Added: 0009150
2013-04-16 17:24 gasche Relationship added related to 0005935
2013-04-16 17:32 frisch Note Added: 0009151
2013-04-21 16:54 jacques-henri.jourdan File Added: 0001-PR-0005899-exposing-a-way-to-inspect-current-call-st.patch
2013-04-21 17:01 jacques-henri.jourdan Note Added: 0009169
2013-04-21 19:07 gasche Note Added: 0009170
2013-04-21 19:13 gasche Note Edited: 0009170 View Revisions
2013-04-22 12:12 jacques-henri.jourdan File Deleted: 0001-PR-0005899-exposing-a-way-to-inspect-current-call-st.patch
2013-04-22 12:13 jacques-henri.jourdan File Added: 0001-PR-0005899-exposing-a-way-to-inspect-current-call-st.patch
2013-04-22 12:14 jacques-henri.jourdan Note Added: 0009178
2013-04-22 12:17 frisch Note Added: 0009179
2013-04-22 13:49 gasche Note Added: 0009182
2013-04-22 14:05 frisch Note Added: 0009183
2013-04-22 14:06 jacques-henri.jourdan File Deleted: 0001-PR-0005899-exposing-a-way-to-inspect-current-call-st.patch
2013-04-22 14:07 jacques-henri.jourdan File Added: 0001-PR-0005899-exposing-a-way-to-inspect-current-call-st.patch
2013-04-22 14:21 jacques-henri.jourdan Note Added: 0009184
2013-04-22 15:23 gasche Note Added: 0009185
2013-04-23 11:52 frisch Note Added: 0009192
2013-05-27 11:20 jacques-henri.jourdan Note Added: 0009356
2013-06-03 22:37 frisch Note Added: 0009386
2013-06-04 07:56 gasche Note Added: 0009387
2013-06-13 10:17 gasche Note Added: 0009473
2013-06-15 23:11 gasche File Added: caml_stash_current_backtrace.diff
2013-06-15 23:11 gasche File Added: printexc_get_callstack.diff
2013-06-15 23:28 gasche Note Added: 0009506
2013-06-16 00:16 dbuenzli Note Added: 0009507
2013-06-16 11:33 gasche Note Added: 0009508
2013-06-17 09:43 frisch Note Added: 0009528
2013-06-17 09:45 frisch Note Edited: 0009528 View Revisions
2013-06-17 16:23 jacques-henri.jourdan Note Added: 0009538
2013-06-19 16:58 gasche Note Added: 0009566
2013-06-19 18:59 frisch Note Added: 0009568
2013-06-19 19:12 jacques-henri.jourdan Note Added: 0009569
2013-06-19 19:44 frisch Note Added: 0009572
2013-06-19 22:29 gasche Note Added: 0009581
2013-07-08 12:12 frisch Note Added: 0009720
2013-07-10 19:18 gasche Note Added: 0009746
2013-07-10 19:42 frisch Note Added: 0009748
2013-07-11 14:37 frisch Note Added: 0009750
2013-12-16 14:06 doligez Tag Attached: patch
2014-01-29 14:31 dbuenzli Note Added: 0010860
2014-02-28 10:26 frisch Note Added: 0010981
2014-03-21 16:17 jacques-henri.jourdan Note Added: 0011074
2014-03-21 16:18 jacques-henri.jourdan File Added: patch_backtraces.diff
2014-03-21 16:30 dbuenzli Note Added: 0011075
2014-03-21 17:32 gasche Note Added: 0011076
2014-03-21 17:32 gasche Note Edited: 0011076 View Revisions
2014-03-21 17:53 dbuenzli Note Added: 0011078
2014-03-24 15:40 frisch Note Added: 0011090
2014-03-28 10:45 frisch Relationship added related to 0006302
2014-03-28 10:49 frisch Note Added: 0011125
2014-04-02 23:10 frisch Target Version => 4.02.0+dev
2014-04-04 16:48 jacques-henri.jourdan File Deleted: current_call_stack.diff
2014-04-04 16:48 jacques-henri.jourdan File Deleted: 0001-PR-0005899-exposing-a-way-to-inspect-current-call-st.patch
2014-04-04 16:48 jacques-henri.jourdan File Deleted: caml_stash_current_backtrace.diff
2014-04-04 16:48 jacques-henri.jourdan File Deleted: printexc_get_callstack.diff
2014-04-04 16:48 jacques-henri.jourdan File Deleted: patch_backtraces.diff
2014-04-04 16:48 jacques-henri.jourdan File Added: patch_backtraces.diff
2014-04-04 16:50 jacques-henri.jourdan Note Added: 0011225
2014-04-08 15:25 gasche Note Added: 0011244
2014-05-05 15:23 frisch Note Added: 0011353
2014-05-05 15:40 jacques-henri.jourdan Note Added: 0011354
2014-05-05 15:41 jacques-henri.jourdan File Added: patch
2014-05-05 15:51 frisch Note Added: 0011355
2014-05-05 16:05 dbuenzli Note Added: 0011356
2014-05-06 10:43 jacques-henri.jourdan Note Added: 0011359
2014-05-06 10:43 jacques-henri.jourdan File Deleted: patch_backtraces.diff
2014-05-06 10:43 jacques-henri.jourdan File Deleted: patch
2014-05-06 10:43 jacques-henri.jourdan File Added: patch
2014-05-06 11:03 dbuenzli Note Added: 0011360
2014-05-06 11:31 jacques-henri.jourdan Note Added: 0011361
2014-05-06 12:08 dbuenzli Note Added: 0011362
2014-05-10 21:41 gasche Note Added: 0011419
2014-05-10 21:41 gasche Status assigned => resolved
2014-05-10 21:41 gasche Resolution open => fixed
2015-12-11 19:26 xleroy Status resolved => closed
2017-02-23 16:43 doligez Category OCaml standard library => standard library

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker