{"action":"created","comment":{"url":"https://api.github.com/repos/ocaml/ocaml/pulls/comments/63679774","id":63679774,"diff_hunk":"@@ -36,22 +36,39 @@ struct caml_loc_info {\n   int loc_lnum;\n   int loc_startchr;\n   int loc_endchr;\n+  int loc_is_inlined;\n };\n \n+/* When compiling with -g, backtrace slots have debug info associated.\n+ * When a call is inlined in native mode, debuginfos form a linked list.\n+ */\n+typedef void * debuginfo;\n+\n /* Check availability of debug information before extracting a trace.\n  * Relevant for bytecode, always true for native code. */\n int caml_debug_info_available(void);\n \n+/* Return debuginfo associated to a slot or NULL. */\n+debuginfo caml_debuginfo_extract(backtrace_slot slot);\n+\n+/* In case of an inlined call return next debuginfo or NULL otherwise. */\n+debuginfo caml_debuginfo_next(debuginfo dbg);\n+\n /* Extract locations from backtrace_slot */\n-void caml_extract_location_info(backtrace_slot pc,\n-                                /*out*/ struct caml_loc_info * li);\n-\n-/* Expose a [backtrace_slot] as a OCaml value of type [raw_backtrace_slot].\n- * The value returned should be an immediate and not an OCaml block, so that it\n- * is safe to store using direct assignment and [Field], and not [Store_field] /\n- * [caml_modify].  */\n-value caml_val_raw_backtrace_slot(backtrace_slot pc);\n-backtrace_slot caml_raw_backtrace_slot_val(value slot);\n+void caml_debuginfo_location(debuginfo dbg, /*out*/ struct caml_loc_info * li);\n+\n+/* In order to prevent the GC from walking through the debug\n+   information (which have no headers), we transform slots to 31/63 bits\n+   ocaml integers by shifting them by 1 to the right. We do not lose\n+   information as slots are aligned.\n+\n+   In particular, we do not need to use [caml_modify] when setting\n+   an array element with such a value.\n+\n+   These functions might be removed in the future.\n+ */","path":"byterun/caml/backtrace_prim.h","position":43,"original_position":43,"commit_id":"c00ca850dcd2d8c7a27395d85406e3cc7c4e91a9","original_commit_id":"c00ca850dcd2d8c7a27395d85406e3cc7c4e91a9","user":{"login":"mshinwell","id":1315488,"avatar_url":"https://avatars.githubusercontent.com/u/1315488?v=3","gravatar_id":"","url":"https://api.github.com/users/mshinwell","html_url":"https://github.com/mshinwell","followers_url":"https://api.github.com/users/mshinwell/followers","following_url":"https://api.github.com/users/mshinwell/following{/other_user}","gists_url":"https://api.github.com/users/mshinwell/gists{/gist_id}","starred_url":"https://api.github.com/users/mshinwell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mshinwell/subscriptions","organizations_url":"https://api.github.com/users/mshinwell/orgs","repos_url":"https://api.github.com/users/mshinwell/repos","events_url":"https://api.github.com/users/mshinwell/events{/privacy}","received_events_url":"https://api.github.com/users/mshinwell/received_events","type":"User","site_admin":false},"body":"Can you elaborate on why these functions might be removed?","created_at":"2016-05-18T10:28:52Z","updated_at":"2016-05-18T10:28:52Z","html_url":"https://github.com/ocaml/ocaml/pull/247#discussion_r63679774","pull_request_url":"https://api.github.com/repos/ocaml/ocaml/pulls/247","_links":{"self":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/comments/63679774"},"html":{"href":"https://github.com/ocaml/ocaml/pull/247#discussion_r63679774"},"pull_request":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/247"}}},"pull_request":{"url":"https://api.github.com/repos/ocaml/ocaml/pulls/247","id":46077601,"html_url":"https://github.com/ocaml/ocaml/pull/247","diff_url":"https://github.com/ocaml/ocaml/pull/247.diff","patch_url":"https://github.com/ocaml/ocaml/pull/247.patch","issue_url":"https://api.github.com/repos/ocaml/ocaml/issues/247","number":247,"state":"open","locked":false,"title":"Make backtraces aware of inlining","user":{"login":"let-def","id":1048096,"avatar_url":"https://avatars.githubusercontent.com/u/1048096?v=3","gravatar_id":"","url":"https://api.github.com/users/let-def","html_url":"https://github.com/let-def","followers_url":"https://api.github.com/users/let-def/followers","following_url":"https://api.github.com/users/let-def/following{/other_user}","gists_url":"https://api.github.com/users/let-def/gists{/gist_id}","starred_url":"https://api.github.com/users/let-def/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/let-def/subscriptions","organizations_url":"https://api.github.com/users/let-def/orgs","repos_url":"https://api.github.com/users/let-def/repos","events_url":"https://api.github.com/users/let-def/events{/privacy}","received_events_url":"https://api.github.com/users/let-def/received_events","type":"User","site_admin":false},"body":"This pull request makes the backtrace infrastructure robust to inlining.\r\nIt already improves the developer experience, but will be even more important with the stronger inlining that is coming.\r\n\r\nWhile it should be usable as it is, I would like to discuss two points to improve before considering a merge.\r\n\r\n### Printexc API\r\n\r\nPrintexc allows the user to observe a backtrace mainly by offering random access to \"slots\".\r\nAn \"O(1)\" cost is implicitly assumed, but not really practical: for instance with inlining, there are more frames than slots in the backtrace buffer.\r\nFlattening the array is required before indexing, which in turns require to distinguish between normal frames and inlined frames in the raw\\_backtrace, etc.\r\n\r\nThe patch tries to hide all differences, but the implementation could be significantly simpler with a few changes.\r\nAs @gasche already suggested that we could cleanup Printexc, my idea would be to:\r\n- deprecate random access, and either make it only observe the 'physical backtrace' (without inlined frames), or explicitly not \"O(1)\"\r\n- use iter-like functions as the main way to observe the trace.\r\n\r\nAlso, coming up with better names and a better 'story' than (raw) backtrace/slot could be helpful for users; though I don't know what other opinions are and I have nothing to suggest :).\r\n\r\n### Representation of backtraces\r\n\r\nIn the mainline OCaml compiler, debug information are stored in a packed way at the end of each frame descriptor.\r\nDebug information fits in a word, using relative adressing to store the filename and bit twiddling to pack coordinates.\r\n\r\nMy understanding is that this was done to maximize locality, efficient traversal of frame table being critical for GC performance.\r\nHowever I would like to simplify representation by adding an indirection (just a pointer) to access the debug data.\r\nThese are not needed on critical path, so indirection shouldn't affect performance. Frame descriptor size is unchanged.\r\n\r\nDebug information would be a few bytes larger, but the representation would be simpler and more uniform (in this case, debug info are extended with a linked list of inlined frames).","created_at":"2015-09-26T07:45:35Z","updated_at":"2016-05-18T10:28:52Z","closed_at":null,"merged_at":null,"merge_commit_sha":"5f439683cfc12378910e588f67736aecb6d8da2f","assignee":null,"milestone":{"url":"https://api.github.com/repos/ocaml/ocaml/milestones/2","html_url":"https://github.com/ocaml/ocaml/milestones/4.04","labels_url":"https://api.github.com/repos/ocaml/ocaml/milestones/2/labels","id":1464889,"number":2,"title":"4.04","description":"","creator":{"login":"damiendoligez","id":19104,"avatar_url":"https://avatars.githubusercontent.com/u/19104?v=3","gravatar_id":"","url":"https://api.github.com/users/damiendoligez","html_url":"https://github.com/damiendoligez","followers_url":"https://api.github.com/users/damiendoligez/followers","following_url":"https://api.github.com/users/damiendoligez/following{/other_user}","gists_url":"https://api.github.com/users/damiendoligez/gists{/gist_id}","starred_url":"https://api.github.com/users/damiendoligez/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/damiendoligez/subscriptions","organizations_url":"https://api.github.com/users/damiendoligez/orgs","repos_url":"https://api.github.com/users/damiendoligez/repos","events_url":"https://api.github.com/users/damiendoligez/events{/privacy}","received_events_url":"https://api.github.com/users/damiendoligez/received_events","type":"User","site_admin":false},"open_issues":83,"closed_issues":28,"state":"open","created_at":"2015-12-17T12:44:22Z","updated_at":"2016-05-17T14:09:55Z","due_on":"2016-07-10T22:00:00Z","closed_at":null},"commits_url":"https://api.github.com/repos/ocaml/ocaml/pulls/247/commits","review_comments_url":"https://api.github.com/repos/ocaml/ocaml/pulls/247/comments","review_comment_url":"https://api.github.com/repos/ocaml/ocaml/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ocaml/ocaml/issues/247/comments","statuses_url":"https://api.github.com/repos/ocaml/ocaml/statuses/c00ca850dcd2d8c7a27395d85406e3cc7c4e91a9","head":{"label":"let-def:inline-backtrace","ref":"inline-backtrace","sha":"c00ca850dcd2d8c7a27395d85406e3cc7c4e91a9","user":{"login":"let-def","id":1048096,"avatar_url":"https://avatars.githubusercontent.com/u/1048096?v=3","gravatar_id":"","url":"https://api.github.com/users/let-def","html_url":"https://github.com/let-def","followers_url":"https://api.github.com/users/let-def/followers","following_url":"https://api.github.com/users/let-def/following{/other_user}","gists_url":"https://api.github.com/users/let-def/gists{/gist_id}","starred_url":"https://api.github.com/users/let-def/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/let-def/subscriptions","organizations_url":"https://api.github.com/users/let-def/orgs","repos_url":"https://api.github.com/users/let-def/repos","events_url":"https://api.github.com/users/let-def/events{/privacy}","received_events_url":"https://api.github.com/users/let-def/received_events","type":"User","site_admin":false},"repo":{"id":16416828,"name":"ocaml","full_name":"let-def/ocaml","owner":{"login":"let-def","id":1048096,"avatar_url":"https://avatars.githubusercontent.com/u/1048096?v=3","gravatar_id":"","url":"https://api.github.com/users/let-def","html_url":"https://github.com/let-def","followers_url":"https://api.github.com/users/let-def/followers","following_url":"https://api.github.com/users/let-def/following{/other_user}","gists_url":"https://api.github.com/users/let-def/gists{/gist_id}","starred_url":"https://api.github.com/users/let-def/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/let-def/subscriptions","organizations_url":"https://api.github.com/users/let-def/orgs","repos_url":"https://api.github.com/users/let-def/repos","events_url":"https://api.github.com/users/let-def/events{/privacy}","received_events_url":"https://api.github.com/users/let-def/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/let-def/ocaml","description":"Read-only mirror of INRIA SVN","fork":true,"url":"https://api.github.com/repos/let-def/ocaml","forks_url":"https://api.github.com/repos/let-def/ocaml/forks","keys_url":"https://api.github.com/repos/let-def/ocaml/keys{/key_id}","collaborators_url":"https://api.github.com/repos/let-def/ocaml/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/let-def/ocaml/teams","hooks_url":"https://api.github.com/repos/let-def/ocaml/hooks","issue_events_url":"https://api.github.com/repos/let-def/ocaml/issues/events{/number}","events_url":"https://api.github.com/repos/let-def/ocaml/events","assignees_url":"https://api.github.com/repos/let-def/ocaml/assignees{/user}","branches_url":"https://api.github.com/repos/let-def/ocaml/branches{/branch}","tags_url":"https://api.github.com/repos/let-def/ocaml/tags","blobs_url":"https://api.github.com/repos/let-def/ocaml/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/let-def/ocaml/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/let-def/ocaml/git/refs{/sha}","trees_url":"https://api.github.com/repos/let-def/ocaml/git/trees{/sha}","statuses_url":"https://api.github.com/repos/let-def/ocaml/statuses/{sha}","languages_url":"https://api.github.com/repos/let-def/ocaml/languages","stargazers_url":"https://api.github.com/repos/let-def/ocaml/stargazers","contributors_url":"https://api.github.com/repos/let-def/ocaml/contributors","subscribers_url":"https://api.github.com/repos/let-def/ocaml/subscribers","subscription_url":"https://api.github.com/repos/let-def/ocaml/subscription","commits_url":"https://api.github.com/repos/let-def/ocaml/commits{/sha}","git_commits_url":"https://api.github.com/repos/let-def/ocaml/git/commits{/sha}","comments_url":"https://api.github.com/repos/let-def/ocaml/comments{/number}","issue_comment_url":"https://api.github.com/repos/let-def/ocaml/issues/comments{/number}","contents_url":"https://api.github.com/repos/let-def/ocaml/contents/{+path}","compare_url":"https://api.github.com/repos/let-def/ocaml/compare/{base}...{head}","merges_url":"https://api.github.com/repos/let-def/ocaml/merges","archive_url":"https://api.github.com/repos/let-def/ocaml/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/let-def/ocaml/downloads","issues_url":"https://api.github.com/repos/let-def/ocaml/issues{/number}","pulls_url":"https://api.github.com/repos/let-def/ocaml/pulls{/number}","milestones_url":"https://api.github.com/repos/let-def/ocaml/milestones{/number}","notifications_url":"https://api.github.com/repos/let-def/ocaml/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/let-def/ocaml/labels{/name}","releases_url":"https://api.github.com/repos/let-def/ocaml/releases{/id}","deployments_url":"https://api.github.com/repos/let-def/ocaml/deployments","created_at":"2014-01-31T18:43:18Z","updated_at":"2015-05-05T08:53:56Z","pushed_at":"2016-05-18T09:31:48Z","git_url":"git://github.com/let-def/ocaml.git","ssh_url":"git@github.com:let-def/ocaml.git","clone_url":"https://github.com/let-def/ocaml.git","svn_url":"https://github.com/let-def/ocaml","homepage":null,"size":178512,"stargazers_count":4,"watchers_count":4,"language":"OCaml","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":4,"default_branch":"trunk"}},"base":{"label":"ocaml:trunk","ref":"trunk","sha":"65722c5ba17d4a11d0ab944f75fa3965f56af001","user":{"login":"ocaml","id":1841483,"avatar_url":"https://avatars.githubusercontent.com/u/1841483?v=3","gravatar_id":"","url":"https://api.github.com/users/ocaml","html_url":"https://github.com/ocaml","followers_url":"https://api.github.com/users/ocaml/followers","following_url":"https://api.github.com/users/ocaml/following{/other_user}","gists_url":"https://api.github.com/users/ocaml/gists{/gist_id}","starred_url":"https://api.github.com/users/ocaml/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ocaml/subscriptions","organizations_url":"https://api.github.com/users/ocaml/orgs","repos_url":"https://api.github.com/users/ocaml/repos","events_url":"https://api.github.com/users/ocaml/events{/privacy}","received_events_url":"https://api.github.com/users/ocaml/received_events","type":"Organization","site_admin":false},"repo":{"id":6786166,"name":"ocaml","full_name":"ocaml/ocaml","owner":{"login":"ocaml","id":1841483,"avatar_url":"https://avatars.githubusercontent.com/u/1841483?v=3","gravatar_id":"","url":"https://api.github.com/users/ocaml","html_url":"https://github.com/ocaml","followers_url":"https://api.github.com/users/ocaml/followers","following_url":"https://api.github.com/users/ocaml/following{/other_user}","gists_url":"https://api.github.com/users/ocaml/gists{/gist_id}","starred_url":"https://api.github.com/users/ocaml/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ocaml/subscriptions","organizations_url":"https://api.github.com/users/ocaml/orgs","repos_url":"https://api.github.com/users/ocaml/repos","events_url":"https://api.github.com/users/ocaml/events{/privacy}","received_events_url":"https://api.github.com/users/ocaml/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ocaml/ocaml","description":"The core OCaml system: compilers, runtime system, base libraries","fork":false,"url":"https://api.github.com/repos/ocaml/ocaml","forks_url":"https://api.github.com/repos/ocaml/ocaml/forks","keys_url":"https://api.github.com/repos/ocaml/ocaml/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ocaml/ocaml/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ocaml/ocaml/teams","hooks_url":"https://api.github.com/repos/ocaml/ocaml/hooks","issue_events_url":"https://api.github.com/repos/ocaml/ocaml/issues/events{/number}","events_url":"https://api.github.com/repos/ocaml/ocaml/events","assignees_url":"https://api.github.com/repos/ocaml/ocaml/assignees{/user}","branches_url":"https://api.github.com/repos/ocaml/ocaml/branches{/branch}","tags_url":"https://api.github.com/repos/ocaml/ocaml/tags","blobs_url":"https://api.github.com/repos/ocaml/ocaml/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ocaml/ocaml/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ocaml/ocaml/git/refs{/sha}","trees_url":"https://api.github.com/repos/ocaml/ocaml/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ocaml/ocaml/statuses/{sha}","languages_url":"https://api.github.com/repos/ocaml/ocaml/languages","stargazers_url":"https://api.github.com/repos/ocaml/ocaml/stargazers","contributors_url":"https://api.github.com/repos/ocaml/ocaml/contributors","subscribers_url":"https://api.github.com/repos/ocaml/ocaml/subscribers","subscription_url":"https://api.github.com/repos/ocaml/ocaml/subscription","commits_url":"https://api.github.com/repos/ocaml/ocaml/commits{/sha}","git_commits_url":"https://api.github.com/repos/ocaml/ocaml/git/commits{/sha}","comments_url":"https://api.github.com/repos/ocaml/ocaml/comments{/number}","issue_comment_url":"https://api.github.com/repos/ocaml/ocaml/issues/comments{/number}","contents_url":"https://api.github.com/repos/ocaml/ocaml/contents/{+path}","compare_url":"https://api.github.com/repos/ocaml/ocaml/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ocaml/ocaml/merges","archive_url":"https://api.github.com/repos/ocaml/ocaml/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ocaml/ocaml/downloads","issues_url":"https://api.github.com/repos/ocaml/ocaml/issues{/number}","pulls_url":"https://api.github.com/repos/ocaml/ocaml/pulls{/number}","milestones_url":"https://api.github.com/repos/ocaml/ocaml/milestones{/number}","notifications_url":"https://api.github.com/repos/ocaml/ocaml/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ocaml/ocaml/labels{/name}","releases_url":"https://api.github.com/repos/ocaml/ocaml/releases{/id}","deployments_url":"https://api.github.com/repos/ocaml/ocaml/deployments","created_at":"2012-11-20T22:18:22Z","updated_at":"2016-05-18T06:21:03Z","pushed_at":"2016-05-18T09:31:49Z","git_url":"git://github.com/ocaml/ocaml.git","ssh_url":"git@github.com:ocaml/ocaml.git","clone_url":"https://github.com/ocaml/ocaml.git","svn_url":"https://github.com/ocaml/ocaml","homepage":"","size":233562,"stargazers_count":770,"watchers_count":770,"language":"OCaml","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":249,"mirror_url":null,"open_issues_count":94,"forks":249,"open_issues":94,"watchers":770,"default_branch":"trunk"}},"_links":{"self":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/247"},"html":{"href":"https://github.com/ocaml/ocaml/pull/247"},"issue":{"href":"https://api.github.com/repos/ocaml/ocaml/issues/247"},"comments":{"href":"https://api.github.com/repos/ocaml/ocaml/issues/247/comments"},"review_comments":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/247/comments"},"review_comment":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ocaml/ocaml/pulls/247/commits"},"statuses":{"href":"https://api.github.com/repos/ocaml/ocaml/statuses/c00ca850dcd2d8c7a27395d85406e3cc7c4e91a9"}}}}